Eliminate some warnings in CoMD #22973
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update CoMD's AccumStencilDist to eliminate recent deprecation warnings, especially from chapel-lang#22784 and chapel-lang#22858. CoMD's prediff filters out warnings so newly-introduced warnings go unnoticed in paratest testing. While there, make some more changes that make CoMD's AccumStencilDist closer to our standard StencilDist. In particular, change a couple of occurrences of `.low` and `.high` to `.lowBound` and `.highBound`. The corresponding changes in StencilDist were made in chapel-lang#19820 when the warnings on `.low` / `.high` were enabled. Now `.low` / `.high` no longer generate warnings, however have a (slightly) different meaning than they had before chapel-lang#19820. Future work: * Should we changes more occurrences of `.low` / `.high` to `.lowBound` / `.highBound` following the changes to StencilDist in chapel-lang#19820? * Eliminate deprecation warnings about TimeUnits and getCurrentTime() in the llnl version. * Perhaps the `.prediff` for CoMD should not filter out warnings any more? Implementation-wise, it means that these scripts now should include warnings, if any, in their output. Signed-off-by: Vassily Litvinov <[email protected]>
benharsh
approved these changes
Aug 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update CoMD's AccumStencilDist to eliminate recent deprecation warnings, especially from #22784 and #22858, following the changes to our standard StencilDist in those PRs. CoMD's prediff filters out warnings so newly-introduced warnings go unnoticed in paratest testing.
While there, make some more changes that make CoMD's AccumStencilDist closer to our standard StencilDist.
In particular, change a couple of occurrences of
.lowand.highto.lowBoundand.highBound. The corresponding changes in StencilDist were made in #19820 when the warnings on.low/.highwere enabled. Now.low/.highno longer generate warnings, however have a (slightly) different meaning than they had before #19820.Future work:
Should we changes more occurrences of
.low/.highto.lowBound/.highBoundfollowing the changes to StencilDist in Start towards making 'low'/'high' return aligned bounds by default (warn + opt-in) #19820?Eliminate deprecation warnings about TimeUnits and getCurrentTime() in the llnl version.
Perhaps the
.predifffor CoMD should not filter out warnings any more? Implementation-wise, it means that these scripts now should include warnings, if any, in their output.Testing: test/studies/comd/elegant/arrayOfStructs/CoMD.chpl passes in the standard and gasnet configurations.