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 option to allow Lumberjack to own communicator #1513

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

gberg617
Copy link
Contributor

@gberg617 gberg617 commented Mar 5, 2025

Summary

  • This PR is a feature
  • This PR adds the ability for Lumberjack to directly own a Communicator object. When this case is true, Lumberjack will call delete on the communicator object when finalize() is called.

@rhornung67
Copy link
Member

setCommunicator() method needs to be added. Also, should initialize() check to make sure it's not being called more than it should? @white238 any additional comments?

Copy link
Contributor

@gunney1 gunney1 left a comment

Choose a reason for hiding this comment

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

Looks okay to me pending the comment of @white238 .

@gberg617
Copy link
Contributor Author

To Address comments from @white238, the following were added to this PR:

  1. Methods in Lumberjack to get and swap communicators, as well as a getter for the isCommunicatorOwned flag.
  2. isInitialized flag added to enforce initialize() to be called only once per Lumberjack instance.
  3. Unit tests to check correct behavior for swapping owned and non-owned communicators in Lumberjack
  4. Documentation
  5. Release notes

@bmhan12
Copy link
Contributor

bmhan12 commented Mar 12, 2025

  1. Documentation

👍
Updated documentation for this PR can be found here: https://axom.readthedocs.io/en/feature-bergel1-lumberjack_set_communicator/axom/lumberjack/docs/sphinx/lumberjack_class.html

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.

4 participants