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 align_extent(), to align extent but not min #5829

Merged
merged 3 commits into from
Mar 27, 2021

Conversation

steven-johnson
Copy link
Contributor

This can be handy when you have an intermediate Func that is being tiled inside an outer Func and you want to ensure that it fits an exact multiple of tiles.

This can be handy when you have an intermediate Func that is being tiled inside an outer Func and you want to ensure that it fits an exact multiple of tiles.
@steven-johnson steven-johnson added enhancement New user-visible features or improvements to existing features. release_notes For changes that may warrant a note in README for official releases. labels Mar 20, 2021
Copy link
Contributor

@dsharletg dsharletg left a comment

Choose a reason for hiding this comment

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

This seems OK to me, but @abadams should take a look. I'm not 100% sure that aligning bounds to a modulus without caring about the remainder should mean that the extent is a multiple of the modulus? But it does seem reasonable.

@abadams
Copy link
Member

abadams commented Mar 22, 2021

The within-compiler representation seems reasonable, but I think it would be better to change the API to be align_extent for this case (to mimic bound_extent)

@steven-johnson
Copy link
Contributor Author

This ended up not being as useful as I thought it would be for my purposes. Adding align_extent sounds fine if we want to land this, but without a clear and present need, do we want to?

@steven-johnson steven-johnson changed the title Allow align_bounds() to align extent but not min Add align_extent(), to align extent but not min Mar 24, 2021
@steven-johnson
Copy link
Contributor Author

Still not sure if this is genuinely needed, but moved to a separate method as suggested.

@abadams
Copy link
Member

abadams commented Mar 26, 2021

I'm sure it will come up as useful at some point, and it's simple enough.

@steven-johnson steven-johnson merged commit 4f152f3 into master Mar 27, 2021
@steven-johnson steven-johnson deleted the srj/align-bounds branch March 27, 2021 23:16
@alexreinking alexreinking added this to the v12.0.0 milestone May 19, 2021
@alexreinking alexreinking removed the release_notes For changes that may warrant a note in README for official releases. label Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New user-visible features or improvements to existing features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants