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

Eliminate some warnings in CoMD #22973

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

vasslitvinov
Copy link
Member

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 .low and .high to .lowBound and .highBound. The corresponding changes in StencilDist were made in #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 #19820.

Future work:

  • Should we changes more occurrences of .low / .high to .lowBound / .highBound following 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 .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.

Testing: test/studies/comd/elegant/arrayOfStructs/CoMD.chpl passes in the standard and gasnet configurations.

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 <vasslitvinov@users.noreply.github.com>
@vasslitvinov vasslitvinov merged commit b40fc62 into chapel-lang:main Aug 15, 2023
7 checks passed
@vasslitvinov vasslitvinov deleted the CoMD-warnings branch August 15, 2023 23:28
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