Skip to content

Field checksum update#173

Draft
mwaxmonsky wants to merge 61 commits into
TURBO-ESM:mainfrom
mwaxmonsky:field_checksum_update
Draft

Field checksum update#173
mwaxmonsky wants to merge 61 commits into
TURBO-ESM:mainfrom
mwaxmonsky:field_checksum_update

Conversation

@mwaxmonsky
Copy link
Copy Markdown
Collaborator

Description

Updates C++ implementation/API to pure AMReX based instead of C/Fortran-based.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code maintenance (refactoring, optimization, etc.)
  • CI/CD changes

Related Issues

Fixes #

Changes Made

  • Updates TIM submodule with C++/AMReX implementation of field_checksum.

Testing

Test Environment

  • Tested on derecho
  • Tested locally
  • Other: _______________

Compiler(s) Tested

  • Intel
  • GNU
  • NVHPC
  • Other: _______________

Test Cases

  • All existing tests pass
  • New tests added and pass
  • Manual testing performed
  • Examples run successfully

Test Results

Performance Impact

  • No performance impact expected
  • Performance improvement
  • Potential performance impact (explain below)

Documentation

  • Code comments updated
  • README updated
  • User documentation updated
  • Developer documentation updated
  • No documentation changes needed

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Additional Notes

@mwaxmonsky mwaxmonsky marked this pull request as ready for review March 4, 2026 22:12
@mwaxmonsky mwaxmonsky requested review from hctorres and johnmauff March 4, 2026 22:12
@johnmauff
Copy link
Copy Markdown
Collaborator

Do you want my review on this PR, or the changes to the TIM library?

@mwaxmonsky
Copy link
Copy Markdown
Collaborator Author

Do you want my review on this PR, or the changes to the TIM library?

My apologies! I forgot to update the TIM PR to be out of draft as well. Probably there is a better place.

Copy link
Copy Markdown
Collaborator

@johnmauff johnmauff left a comment

Choose a reason for hiding this comment

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

@mwaxmonsky While I like where you are going with respect to using more AMReX data-structures, I think that more can be done to make it more modular and reusable. In particular, I would like to see it support multifab arrays. So keep the lower level function that operates over a box and an Array4. Add a method that loops over element of a multifab calling the lower level function. After the on-MPI rank calculations are performed then perform the parallel sum. I do like the way you are wrapping the Fortran memory into a box and an Array4. I am wondering about the construction of your box and Array4 in the interface layer. Should the array bounds go from 0 to size_array-1?

@mwaxmonsky
Copy link
Copy Markdown
Collaborator Author

Putting this back into draft until it is re-prioritized.

@mwaxmonsky mwaxmonsky marked this pull request as draft March 27, 2026 15:12
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 3, 2026

👋 Friendly reminder @hctorres — this PR has had no activity for 7 days and is still awaiting your review. When you have a moment, please take a look!

1 similar comment
@github-actions
Copy link
Copy Markdown

👋 Friendly reminder @hctorres — this PR has had no activity for 7 days and is still awaiting your review. When you have a moment, please take a look!

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