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

Bnb/1d nc loading #247

Merged
merged 11 commits into from
Dec 4, 2024
Merged

Bnb/1d nc loading #247

merged 11 commits into from
Dec 4, 2024

Conversation

bnb32
Copy link
Collaborator

@bnb32 bnb32 commented Nov 25, 2024

Some adjustments to loaders so they work nicer with flattened nc data. Also fix for slicer edge case where modulo(grid_shape, fwp_chunk_shape) is too small for generator padding layer requirements.

…pass chunk shape. For some grid sizes and forward pass chunk shapes not forcing this minimum shape can result in a chunk that is too small for the generator. Fixed slice test, which was not checking all slices.
…cing values (hr_cropped_slices, for example) depend a lot on the pad width implementations so these should be grouped.
@bnb32 bnb32 force-pushed the bnb/1d_nc_loading branch from 587c375 to 11acb62 Compare November 25, 2024 22:28
@bnb32 bnb32 requested a review from grantbuster November 26, 2024 15:02
lr_slice_stop,
self.spatial_pad,
)
warn(
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this should almost be an exception and force you to adjust your fwp shape... It could result in some pretty garbage data. Does this increase the chunk size for the last chunk and crop more of the output? And leave the neighboring chunk alone? Actually that makes sense. Does this check vertical chunk dims too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah this pads only the "final" chunk and then crops the output more. This is done for each spatial dimension so it ends up padding the right-most and bottom-most boundary. True edge case here since it almost never happens when you have non-zero spatial padding and then only happens if spatial padding is 1 and the last unpadded chunk size is also 1.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha that makes sense. good solution too!

@bnb32 bnb32 merged commit e38597a into main Dec 4, 2024
12 checks passed
@bnb32 bnb32 deleted the bnb/1d_nc_loading branch December 4, 2024 17:19
github-actions bot pushed a commit that referenced this pull request Dec 4, 2024
@grantbuster grantbuster mentioned this pull request Dec 19, 2024
@bnb32 bnb32 mentioned this pull request Feb 4, 2025
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.

2 participants