Closed aprilnovak closed 4 years ago
I agree that if the capability were to be retained, there should be two separate classes for the reasons you outlined. I'm fine in principle with removing the ability to do surrogate-surrogate coupling, but (correct me if I'm wrong @sphamil) the only place the 2G diffusion solver is used right now is in that class which means the diffusion solver wouldn't be used anywhere.
I thought the goal was to avoid having multiphysics drivers that are specific to certain code couplings. I don't really care about losing the surrogate-surrogate coupling per se, but I feel like it's indicative of us moving away from having a code-agnostic abstraction.
To me, the "code agnostic" part is the abstract base classes like CoupledDriver
, NeutronicsDriver
, etc. that provide the interfaces that specific single-physics classes and coupled classes need to conform to. For the coupled driver, it would be nice if the logic were completely general and the only concrete classes that need to be implemented are the single-physics classes, but I think we still have some work to do to get to that point. That ought to be an end goal though if possible.
I agree completely, we have some work before we get to the code-agnostic coupled classes. I just wanted to make sure that was still the goal...the discussion about dropping the surrogate-surrogate coupling made me think we might be abandoning that. In the early CASL days, there was talk about having a similar system of interoperable codes so that you would be able to switch between subchannel or full CFD, or between diffusion/SN/MC on the neutronics. The problem was that they tried to make the coupling interface so general that it was trying to couple any conceivable set of physics codes and ended up being tremendously complicated to use. They abandoned that and ended up with a series of one-off, code-specific couplings (each with its own executable). I don't want to end up there, even if we have to have some of the code-specific stuff in the near term.
Great, I think we're on the same page here. @aprilnovak is certainly helping us get there piece by piece :)
I think the two-code drivers will be a stepping stone towards what you both have mentioned where we truly can swap out just single-physics drivers in a CoupledDriver
. To get to that point, we'll need to make the data transfer mechanisms more general - such as passing temperatures plus an array of Point
s that describe where those temperatures are located to the neutronics solver, which can internally look up a cell ID without needing 2-code-specific mappings such as d_power_map
.
We'll definitely keep the two-group diffusion in the code base, I'm just proposing for the time being that we remove the #ifdef USE_SHIFT
stuff from Multiphysics_Driver
and renaming it.
This issue is no longer relevant.
The convention in most of this project is to define coupled drivers that only iterate between two codes. i.e.
openmc_nek_driver
iterates between OpenMC and Nek. TheMultiphysics_Driver
class iterates between a surrogate T/H model and either Shift or a two-group diffusion surrogate. For consistency, this class should be split up into two classes, one that couples Shift and the surrogate T/H, and a second that couples surrogate diffusion with surrogate T/H.But, there may not be any motivation for coupling two surrogate models, since the surrogates are intended for debugging or coupling acceleration in the future. So, I propose renaming
Multiphysics_Driver
toShiftHeatFluidsDriver
and removing the ability to couple two surrogate models.An alternative would be to split
Multiphysics_Driver
into two classes, with a lot of source code redundancy, since the mappings for Shift and the surrogate neutronics models are very similar.Thoughts @sphamil @paulromano @RonRahaman ?