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 unit testing for Slic ranks and rank counts #1431

Merged
merged 14 commits into from
Oct 7, 2024

Conversation

bmhan12
Copy link
Contributor

@bmhan12 bmhan12 commented Sep 30, 2024

This PR:

  • Adds testing for ranks and rank counts information in slic_macros_parallel unit tests.
  • Remove std::ends from SLIC_ASSERT/SLIC_ASSERT_MSG/SLIC_CHECK/SLIC_CHECK_MSG macros that prevented Lumberjack from combining messages. When packing/unpacking messages for combining, std::ends does not play nice in std::string --> C-style string conversions. Trailing std::ends null character does not get included in the conversion, so output/rank Node's message (with std::ends null character) will always be different from the other nodes (nostd::ends null character), and as such no combining would take place.

Relates to #1410

@bmhan12 bmhan12 added Slic Issues related to Axom's 'slic' component Testing Issues related to testing Axom labels Sep 30, 2024
@bmhan12 bmhan12 force-pushed the bugfix/han12/slic_rank_info branch from 4c734c1 to 41e5ae8 Compare October 1, 2024 16:41
Copy link
Member

@rhornung67 rhornung67 left a comment

Choose a reason for hiding this comment

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

Looks good. The only thought I have is that it may make the slic tests easier to understand if there were comments in the code indicating why the integer constants are chosen as they are; e.g., why N in something like (__LINE__ - N)

Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

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

Thanks @bmhan12 -- please don't forget to update the RELEASE_NOTES

@bmhan12 bmhan12 merged commit e5dcbaa into develop Oct 7, 2024
13 checks passed
@bmhan12 bmhan12 deleted the bugfix/han12/slic_rank_info branch October 7, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Slic Issues related to Axom's 'slic' component Testing Issues related to testing Axom
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants