Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add spatial rescaler #414

Merged
merged 7 commits into from
Aug 14, 2023
Merged

add spatial rescaler #414

merged 7 commits into from
Aug 14, 2023

Conversation

guopengf
Copy link
Contributor

Follow the discussion at #410 and add the spatial rescaler implementation.

@guopengf guopengf linked an issue Jul 27, 2023 that may be closed by this pull request
@guopengf guopengf changed the title 410 add spatial rescaler add spatial rescaler Jul 28, 2023
@guopengf guopengf marked this pull request as ready for review August 3, 2023 15:30
tests/test_encoder_modules.py Outdated Show resolved Hide resolved
generative/networks/blocks/encoder_modules.py Outdated Show resolved Hide resolved
generative/networks/blocks/encoder_modules.py Outdated Show resolved Hide resolved
generative/networks/blocks/encoder_modules.py Outdated Show resolved Hide resolved
@guopengf guopengf requested a review from marksgraham August 10, 2023 03:02
Copy link
Collaborator

@marksgraham marksgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me - just one minor docstring change I noticed!

n_stages: number of interpolation stages.
size: output spatial size (int or Tuple[int] or Tuple[int, int] or Tuple[int, int, int]).
method: algorithm used for sampling.
multiplier: multiplier for spatial size. If scale_factor is a tuple,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this part about scale_factor doesn't make sense - there is no variable with that name, and multiplier can't be a tuple. maybe just delete the part about scale_factor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out the docstring problem. I have fixed it. This remembers me that the multiplier can be a Sequence[float]. I already added this type and a new test case for this situation in the latest commit.

"multiplier": (0.25, 0.5, 0.75),

@guopengf guopengf requested a review from marksgraham August 11, 2023 15:38
Copy link
Collaborator

@marksgraham marksgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for the addition

@marksgraham marksgraham merged commit 11e6323 into main Aug 14, 2023
@marksgraham marksgraham deleted the 410-add-spatial-rescaler branch August 14, 2023 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible Improvements for Current Repository
2 participants