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
234 stars 89 forks source link

Remove operator from reactor #830

Open jakehader opened 2 years ago

jakehader commented 2 years ago

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

john-science commented 2 years ago

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 commented 2 years ago

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"):

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

jakehader commented 2 years ago

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

john-science commented 2 years ago

@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!