Remove Solver template parameter for AdaptiveTimeStepping::step()

[hakonhagland]

Removes the template parameter Solver used with the AdaptiveTimeStepping::solve() method. Try to replace the template parameter with a equivalent Solver value from the Opm::Properties:: namespace. I am currently debugging this, so I will put it in draft mode for now.

[hakonhagland]

jenkins build this please

[hakonhagland]

The jenkins build reports the compile time error:

/var/lib/jenkins/workspace/opm-simulators-PR-builder/opm/simulators/flow/SimulatorFullyImplicitBlackoil.hpp:462:58: error: cannot convert ‘Opm::NonlinearSolver<Opm::Properties::TTag::FlowSolventProblem, Opm::BlackoilModel<Opm::Properties::TTag::FlowSolventProblem> >’ to ‘Opm::AdaptiveTimeStepping<Opm::Properties::TTag::FlowSolventProblem>::Solver&’ {aka ‘Opm::NonlinearSolver<Opm::Properties::TTag::FlowSolventProblem, Opm::FIBlackOilModel<Opm::Properties::TTag::FlowSolventProblem> >&’}

Meaning that SimulatorFullyImplicitBlackoil.hpp uses Model = Opm::BlackoilModel<Opm::Properties::TTag::FlowSolventProblem> whereas AdaptiveTimeStepping.hpp uses Model = Opm::FIBlackOilModel<Opm::Properties::TTag::FlowBrineProblem>. If I change the definition of Model in SimulatorFullyImplicitBlackoil.hpp :

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/SimulatorFullyImplicitBlackoil.hpp#L112

to use the Model defined in Opm::Properties like this:

    using Model = GetPropType<TypeTag, Properties::Model>;

and recompile, I get another error

home/hakon/test/opm2/opm/opm-simulators/opm/simulators/flow/FlowMain.hpp:405:114: error: ‘const Opm::SimulatorFullyImplicitBlackoil<Opm::Properties::TTag::FlowBrineProblem>::Model’ {aka ‘const class Opm::FIBlackOilModel<Opm::Properties::TTag::FlowBrineProblem>’} has no member named ‘localAccumulatedReports’
  405 |             printFlowTrailer(mpi_size_, threads, total_setup_time_, deck_read_time_, report, simulator_->model().localAccumulatedReports());
      |                                                                                              ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~

@akva2 Any idea on how to solve this?

[hakonhagland]

By defining Model like this in SimulatorFullyImplicitBlackoil.hpp:

using Model = GetPropType<TypeTag, Properties::Model>;

the compiler will use the definition of Model in FlowProblemBlackoilProperties.hpp:

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/FlowProblemBlackoilProperties.hpp#L65-L66

which uses FIBlackOilModel. Instead we would like to use BlackOilModel defined in blackoilmodel.hh:

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/models/blackoil/blackoilmodel.hh#L273

Note that this uses a capitial "O" in BlackOilModel, whereas the one defined in BlackOilModel.hpp

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/BlackoilModel.hpp#L61

uses a lower case "o" in BlackoilModel. What will be the best way to solve this?

[akva2]

The Model in the typetag is the 'ewoms' (opm-models) type model, while the BlackoilModel.hpp is the opm model which is not the same thing. Welcome to the domain of frankenstein. There is no typetag entry for the latter.

[hakonhagland]

Welcome to the domain of frankenstein.

Yes, indeed :sweat_smile: a scary place to enter

There is no typetag entry for the latter.

Then as a next attempt I would simply try to do the following in AdaptiveTimeStepping.hpp (and avoid using the Model = GetPropType<TypeTag, Properties::Model> type):

#include <opm/simulators/flow/BlackoilModel.hpp>
using Model = BlackoilModel<TypeTag>;
using Solver = NonlinearSolver<TypeTag, Model>;
// ...

Although this actually works for most opm .cpp files, there are cases like flowexp_comp.cpp which includes FlowProblemComp.hpp

https://github.com/OPM/opm-simulators/blob/d2b272b5f54779a72923d090b0a6282ae56e241a/flowexperimental/comp/flowexp_comp.hpp#L28

which includes FlowProblem.hpp

https://github.com/OPM/opm-simulators/blob/d2b272b5f54779a72923d090b0a6282ae56e241a/opm/simulators/flow/FlowProblemComp.hpp#L34

which includes AdaptiveTimeStepping.hpp

https://github.com/OPM/opm-simulators/blob/d2b272b5f54779a72923d090b0a6282ae56e241a/opm/simulators/flow/FlowProblem.hpp#L65

If now AdaptiveTimeStepping.hpp includes BlackoilModel.hpp as proposed above, then it again will include BlackoilModelProperties.hpp

https://github.com/OPM/opm-simulators/blob/d2b272b5f54779a72923d090b0a6282ae56e241a/opm/simulators/flow/BlackoilModel.hpp#L31

which will include FlowProblemBlackoilProperties.hpp

https://github.com/OPM/opm-simulators/blob/d2b272b5f54779a72923d090b0a6282ae56e241a/opm/simulators/flow/BlackoilModelProperties.hpp#L27

which defines the MaterialLaw property with given traits

https://github.com/OPM/opm-simulators/blob/d2b272b5f54779a72923d090b0a6282ae56e241a/opm/simulators/flow/FlowProblemBlackoilProperties.hpp#L76

which will use FluidSystem::waterPhaseIdx to create three phase material traits, however this is not using the expected fluid system for a blackoil problem but rather the fluid system included here

https://github.com/OPM/opm-simulators/blob/d2b272b5f54779a72923d090b0a6282ae56e241a/flowexperimental/comp/flowexp_comp.hpp#L23

which uses a waterPhaseIdx = -1

https://github.com/OPM/opm-common/blob/efbebfe159d41477aa2acffce9c6648a6b587294/opm/material/fluidsystems/GenericOilGasFluidSystem.hpp#L69

which leads to the following compilation error

[...]
/home/hakon/test/opm2/opm/opm-simulators/flowexperimental/comp/flowexp_comp.cpp:83:35:   required from here
/home/hakon/test/opm2/opm/opm-common/opm/material/fluidmatrixinteractions/MaterialTraits.hpp:105:21: error: static assertion failed: wettingPhaseIdx is out of range
  105 |     static_assert(0 <= wettingPhaseIdx && wettingPhaseIdx < numPhases,
      |                   ~~^~~~~~~~~~~~~~~~~~
/home/hakon/test/opm2/opm/opm-common/opm/material/fluidmatrixinteractions/MaterialTraits.hpp:105:21: note: the comparison reduces to ‘(0 <= -1)’

[bska]

Removes the template parameter Solver used with the AdaptiveTimeStepping::solve() method. Try to replace the template parameter with a equivalent Solver value from the Opm::Properties:: namespace.

If you don't mind me asking, what problem are you trying to solve by this? You're taking a specific direction, but I guess I'm missing the bigger picture/context for this change.

[hakonhagland]

what problem are you trying to solve by this?

@bska Good question. In the beginning I just wanted to be able to declare the solver as a class variable of AdaptiveTimeStepping. However, I realized later that that would be difficult due to the serialization of AdaptiveTimeStepper. So currently I think of this as a merely cleanup or simplification of the current code. I was also curious why or if this template parameter was necessary. Now as as the complexity of the problem is clearer to me, we might just leave it there? Though, the difficulty of removing it might indicate some deeper issues with the type/property system that could be worth investigating. What do you think?

[hakonhagland]

while the BlackoilModel.hpp is the opm model

@akva2 There is also path of include files such that if one includes the opm model in BlackoilModel.hpp like what is done in SimulatorFullyImplicitBlackoil.hpp the ewoms model blackoilmodel.hh will be automatically included:

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/SimulatorFullyImplicitBlackoil.hpp#L44

and BlackoilModel.hpp includes BlackoilModelNldd.hpp

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/BlackoilModel.hpp#L30

and BlackoilModelNldd.hpp includes ISTLSolverGpuBridge.hpp

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/BlackoilModelNldd.hpp#L42

and ISTLSolverGpuBridge.hpp includes ISTLSolver.hpp

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/linalg/ISTLSolverGpuBridge.hpp#L25

and ISTLSolver.hpp includes FlowBaseProblemProperties.hpp:

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/linalg/ISTLSolver.hpp#L38

and FlowBaseProblemProperties.hpp includes TracerModel.hpp

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/FlowBaseProblemProperties.hpp#L42

and TracerModel.hpp includes GenericTracerModel.hpp

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/TracerModel.hpp#L35

and GenericTracerModel.hpp includes blackoilmodel.hh

https://github.com/OPM/opm-simulators/blob/6bfb60ded033450d1df31ae79674f429b26a4422/opm/simulators/flow/GenericTracerModel.hpp#L35

I think this is what causes the redefinition of the FluidSystem property for cases like flowexp_comp.cpp since blackoilmodel.hh (re)defines the fluid system property to by of type BlackOilFluidSystem:

https://github.com/OPM/opm-simulators/blob/aa5c52ee5694f3911aa974fd72a749834b93fc25/opm/models/blackoil/blackoilmodel.hh#L141