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

Block dilution #1393

Merged
merged 10 commits into from
Oct 30, 2023
Merged

Block dilution #1393

merged 10 commits into from
Oct 30, 2023

Conversation

maddyscientist
Copy link
Member

This simple PR add supports for local disjoint hyper-cubic block dilution. I developed this as part of some experiments with smoothers. This effort is on ice, but since this is a nice self-contained feature addition, I figured it's best to get this upstreamed. This work makes some small changes to the colorspinor::FloatNOrder accessor, which will be helpful for multi-RHS.

Note this PR is dependent on #1384 and so should not be considered for merging until after that is merged.

@maddyscientist maddyscientist requested review from a team as code owners July 18, 2023 20:36
@maddyscientist maddyscientist changed the title Feature/dilution smoother Block dilution Jul 18, 2023
@maddyscientist maddyscientist mentioned this pull request Oct 20, 2023
2 tasks
@weinbe2
Copy link
Contributor

weinbe2 commented Oct 27, 2023

This is a nice PR @maddyscientist , thanks!

One concern I have is that, when running dilution_test by hand, there's an OOM error (on an 80GB card). This is reasonable on the merits because the default values are a volume of 24x24x24x24 and the default dilution size is 1x1x1x1. I know that "under the hood" the maximum number of dilution blocks is 128, but the OOM error hits during the allocations before that.

This is not an issue during ctest since the local volume, and the dilution block size, is sufficiently small.

In the interest of the test executables "just working", I recommend changing the default value for --dilution-block-size to 8 8 8 8, since it "just works", though I'm not married to any particular value so long as it doesn't OOM on a reasonable GPU and the number of dilution vectors is <= 128.

Copy link
Contributor

@weinbe2 weinbe2 left a comment

Choose a reason for hiding this comment

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

Pending Jenkins, approved!

@maddyscientist maddyscientist merged commit bfc7ba0 into develop Oct 30, 2023
@maddyscientist maddyscientist deleted the feature/dilution_smoother branch October 30, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants