armi icon indicating copy to clipboard operation
armi copied to clipboard

Remove operator from reactor

Open jakehader opened this issue 3 years ago • 5 comments

There are only a handful of uses of r.o. it would be great to get rid of these, as:

  • it is unclear why a reactor should have an operator
  • reactor construction is muddled by being incomplete until the reactor has an operator
  • many instances are only used for getting a cs
  • removing the r.o would reduce the number of circular references r.o -> o.r -> r.o ...
  • in almost all circumstances, a reactor and an operator are already in scope.

jakehader avatar Aug 11 '22 11:08 jakehader

Yeah, it's never made sense to me that the Reactor object should have an Operator on it.

It really seems like accidental design, not purposeful design.

john-science avatar Aug 11 '22 15:08 john-science

Here are all of the usages of this I could find in ARMI (though, of course, any number of plugins might be using this "feature"):

  • [ ] high-level
armi/interfaces.py:  self.o = r.o if r else None
armi/mpiActions.py:  self.r.o = self.o
  • [ ] operators
armi/operators/operator.py:  r.o = self
armi/operators/operator.py:  self.r.o = self
armi/operators/operator.py:  self.r.o = None
  • [ ] neutronics
armi/physics/neutronics/globalFlux/globalFluxInterface.py:  daysIntoCycle = sum(self.r.o...
armi/physics/neutronics/macroXSGenerationInterface.py:  sum([self.r.o.burnSteps[i] ...
  • [ ] converters
armi/reactor/converters/geometryConverters.py:  self._o = self._sourceReactor.o
armi/reactor/converters/parameterSweeps/generalParameterSweepConverters.py:  fh = r.o.getInterface("fuelHandler")
  • [ ] uniform mesh (recently added back in by PR #961
newReactor.o = (sourceReactor.o)
  • [X] uniform mesh
armi/reactor/converters/uniformMesh.py:  newReactor = Reactor(sourceReactor.o.cs.caseTitle, bp)
armi/reactor/converters/uniformMesh.py:  coreDesign.construct(sourceReactor.o.cs, bp, newReactor, loadAssems=False)
armi/reactor/converters/uniformMesh.py:  _geometryConverter = newReactor.core.growToFullCore(sourceReactor.o.cs)
  • [x] report plotting
armi/utils/reportPlotting.py:  dbi = reactor.o.getInterface("database")
armi/utils/reportPlotting.py:  dd = reactor.o.getInterface("ductDistortion")
armi/utils/reportPlotting.py:  history = reactor.o.getInterface("history")

So, a really cursory read of the situation makes it look like the things that would need to change are:

  • The UniformMesh should keep a copy of the CaseSettings, and not the operator.
  • Clean up of reportPlotting.py
  • paramSweepConverters needs to get the FuelHandlerInterface somehow differently.
  • The neutronics interfaces above need to get steps/stepLength information from a different source.
  • There may be some unwinding of Interfaces and MPI stuff; but it might that is only complicated BECAUSE of this problem.

john-science avatar Aug 11 '22 15:08 john-science

I think this should be open because it was auto closed when the other PR was merged. Thanks for the effort on this @john-science! I hope there aren't too many downstream impacts

jakehader avatar Sep 29 '22 01:09 jakehader

@jakehader So far, there are no downstream impacts. I only did the removed the really low-hanging fruit versions of r.o (that I could fit into my day).

What's left is some higher-level stuff that will have possible downstream impacts. It appears that sometimes people use r.o.getInterface("xxx"), and squashing all usages of those is the trick. There aren't TOO many places people do this, but it's certainly some fast-and-loose code that will take some untangling to improve the code flow.

TLDR; this is just a start. Thanks!

john-science avatar Sep 29 '22 15:09 john-science