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 :

opm-simulators/opm/simulators/flow/SimulatorFullyImplicitBlackoil.hpp

Line 112 in 6bfb60d

using Model = BlackoilModel<TypeTag>;

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:

opm-simulators/opm/simulators/flow/FlowProblemBlackoilProperties.hpp

Lines 65 to 66 in 6bfb60d

	struct Model<TypeTag, TTag::FlowBaseProblemBlackoil>
	{ using type = FIBlackOilModel<TypeTag>; };

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

opm-simulators/opm/models/blackoil/blackoilmodel.hh

Line 273 in 6bfb60d

class BlackOilModel

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

opm-simulators/opm/simulators/flow/BlackoilModel.hpp

Line 61 in 6bfb60d

class BlackoilModel

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 😅 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

opm-simulators/flowexperimental/comp/flowexp_comp.hpp

Line 28 in d2b272b

#include <opm/simulators/flow/FlowProblemComp.hpp>

which includes FlowProblem.hpp

opm-simulators/opm/simulators/flow/FlowProblemComp.hpp

Line 34 in d2b272b

#include <opm/simulators/flow/FlowProblem.hpp>

which includes AdaptiveTimeStepping.hpp

opm-simulators/opm/simulators/flow/FlowProblem.hpp

Line 65 in d2b272b

#include <opm/simulators/timestepping/AdaptiveTimeStepping.hpp>

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

opm-simulators/opm/simulators/flow/BlackoilModel.hpp

Line 31 in d2b272b

#include <opm/simulators/flow/BlackoilModelProperties.hpp>

which will include FlowProblemBlackoilProperties.hpp

opm-simulators/opm/simulators/flow/BlackoilModelProperties.hpp

Line 27 in d2b272b

#include <opm/simulators/flow/FlowProblemBlackoilProperties.hpp>

which defines the MaterialLaw property with given traits

opm-simulators/opm/simulators/flow/FlowProblemBlackoilProperties.hpp

Line 76 in d2b272b

using Traits = ThreePhaseMaterialTraits<Scalar,

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

opm-simulators/flowexperimental/comp/flowexp_comp.hpp

Line 23 in d2b272b

#include <opm/material/fluidsystems/GenericOilGasFluidSystem.hpp>

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:

opm-simulators/opm/simulators/flow/SimulatorFullyImplicitBlackoil.hpp

Line 44 in 6bfb60d

#include <opm/simulators/flow/BlackoilModel.hpp>

and BlackoilModel.hpp includes BlackoilModelNldd.hpp

opm-simulators/opm/simulators/flow/BlackoilModel.hpp

Line 30 in 6bfb60d

#include <opm/simulators/flow/BlackoilModelNldd.hpp>

and BlackoilModelNldd.hpp includes ISTLSolverGpuBridge.hpp

opm-simulators/opm/simulators/flow/BlackoilModelNldd.hpp

Line 42 in 6bfb60d

#include <opm/simulators/linalg/ISTLSolverGpuBridge.hpp>

and ISTLSolverGpuBridge.hpp includes ISTLSolver.hpp

opm-simulators/opm/simulators/linalg/ISTLSolverGpuBridge.hpp

Line 25 in 6bfb60d

#include <opm/simulators/linalg/ISTLSolver.hpp>

and ISTLSolver.hpp includes FlowBaseProblemProperties.hpp:

opm-simulators/opm/simulators/linalg/ISTLSolver.hpp

Line 38 in 6bfb60d

#include <opm/simulators/flow/FlowBaseProblemProperties.hpp>

and FlowBaseProblemProperties.hpp includes TracerModel.hpp

opm-simulators/opm/simulators/flow/FlowBaseProblemProperties.hpp

Line 42 in 6bfb60d

#include <opm/simulators/flow/TracerModel.hpp>

and TracerModel.hpp includes GenericTracerModel.hpp

opm-simulators/opm/simulators/flow/TracerModel.hpp

Line 35 in 6bfb60d

#include <opm/simulators/flow/GenericTracerModel.hpp>

and GenericTracerModel.hpp includes blackoilmodel.hh

opm-simulators/opm/simulators/flow/GenericTracerModel.hpp

Line 35 in 6bfb60d

#include <opm/models/blackoil/blackoilmodel.hh>

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:

opm-simulators/opm/models/blackoil/blackoilmodel.hh

Line 141 in aa5c52e

using type = BlackOilFluidSystem<Scalar>;