terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
212 stars 82 forks source link

Clearing worker processor instances of reactor state after every interaction #1729

Closed zachmprince closed 2 weeks ago

zachmprince commented 2 weeks ago

What is the change?

The PR creates a method in Operator that is called _finalizeInteract which is called inside interface loop of _interactAll. This method is overridden by OperatorMPI to "reset" worker processors by broadcasting a command to force execution of _resetWorker.

Why is the change being made?

The purpose of this change to force worker resets after each interaction is complete, instead of relying on plugins to return the correct mpiActionRequiresReset during MpiAction invocations. The goal was to reduce memory when going to subsequent interfaces that don't require a distributed state. However, the addition of this reset call is not reducing memory on worker processors, which means the "reset" implementation may be flawed. However, the change proposed in this PR is still appropriate in principle and improvements to the reset functionality can be improved in the future.


Checklist

john-science commented 2 weeks ago

Quick Question:

You added new code, why are there no tests of this? (Did you try, and it was too hard to test?)

zachmprince commented 2 weeks ago

Quick Question:

You added new code, why are there no tests of this? (Did you try, and it was too hard to test?)

I just didn't think of it. I can add a test.

zachmprince commented 2 weeks ago

@john-science @opotowsky Changed log message to extra and added a test in test_mpiFeatures.