Re: [OPM] Refactoring of the fully implicit solver class
On 22/05/15 15:43, Jørgen Kvalsvik wrote: On 05/22/2015 01:09 PM, Atgeirr Rasmussen wrote: Dear OPM community, We have been considering how to best refactor the (huge) class FullyImplicitBlackoilSolver in such a manner that it can be more easily be extended with new options and functionality without copying the whole class (as is currently done to implement the flow_polymer variant in opm-polymer). The goal is simple enough: avoid copying of code between the black-oil and black-oil-with-polymer cases, which also will make the code a better base for further extensions. A first step has been taken: separating the NewtonSolver functionality into its own class, leaving the rest of the functionality in the BlackoilModel class. Further refactoring of this is faced with some nontrivial technical challenges, centering on the State classes. For example, many functions take a State object argument, which (in the current approach) are different between models (black-oil and b.o. with polymer for example). Then that function cannot be virtual (it would need to take the same argument). There seems to be two main alternatives: 1. Applying the "curiously recurring template pattern" (it is indeed called that!). This means essentially using templates to get what we want, including the ability to extend models, reusing most functions and only modifying the few that we want to be different. This will all be checked at compile time, so illegal combinations of objects (such as accidentally using a State class without a certain member together with a model that requires that member) will not even compile. The drawback is that it involves a little more boilerplate code than regular inheritance: assuming that we want to an extensible black-oil model and a polymer model that extends it we'll have three classes, like this: template class BlackoilModelBase { // This is where almost all the functions and data go. }; class BlackoilModel : public BlackoilModelBase { // This contains only type definitions and a constructor. }; class BlackoilPolymerModel: public BlackoilModelBase { // Contains type definitions and a constructor. // Contains new functions, as well as new versions of functions from BlackoilModelBase. }; I do think that the boilerplate is manageable, and that people will get by just fine by starting their own extension by copying an existing extension. (The resulting copied code will not be that many lines). This pattern is a fairly common C++ idiom and is used a lot in Dune for example. 2. Using regular inheritance and a generic State class. This means we have a single, flexible State class that can be used for all purposes. It should function a bit like a map>, but not be implemented like that. This solution is most similar to how things would be done in Matlab, or other weakly typed languages: any usage errors would not be caught until runtime. Each access to the State will be a little more annoying than before, instead of state.saturation() you would need to call state.get("saturation") and it could possibly fail. In summary, none of these solutions are perfect, but both are workable. I have discussed this with Andreas, and we agreed to propose alternative 1 as the one we'd like to try out first. I would like to hear your opinions! Atgeirr The third option, which is somewhat similar to the first option (which by the way is much better than the broken common State class idiom) is to properly split things up into values and transactions and abandon the idea of a full class that is initialised, then called solve() upon. This means more flexibility in terms of replacing, plugging in and extending individual parts, as functions could be either arguments, static (aka hard coded) or compile-time arguments (hello templates), with the added benefit that you can replace sub components as containers with relative ease. Additionally, this actually opens for concurrency, something we should strive for in design considering the state we're in now. And if someone should still prefer having a class manage everything with init => solve, then this class would now be trivial to write. this is a good idea, although i dunno what would be involved here in achieving it. as an example, somewhat on a sidetrack but to show how such an approach can be used in codes. it's a bit hard to make this understandable in email form, but consider this template definition for a RANS driver: /*! \brief Driver class for RANS simulators. \details A RANS simulator is a coupling between a fluid solver and a viscosity solver. The coupling may be segregated or semi-implicit. */ template class Coupling=SIMCoupled> class SIMRANS : public Coupling a RANS coupling is a general pattern; you have some fluid (navier-stokes) solver which provides a velocity field to a viscosity solver, which again provides a viscosity field to the fluid solver. we can then instance this template for various fluid solver (G2
Re: [OPM] Refactoring of the fully implicit solver class
On 05/22/2015 01:09 PM, Atgeirr Rasmussen wrote: > Dear OPM community, > > We have been considering how to best refactor the (huge) class > FullyImplicitBlackoilSolver > in such a manner that it can be more easily be extended with new options and > functionality > without copying the whole class (as is currently done to implement the > flow_polymer variant > in opm-polymer). > > The goal is simple enough: avoid copying of code between the black-oil and > black-oil-with-polymer > cases, which also will make the code a better base for further extensions. A > first step has been taken: > separating the NewtonSolver functionality into its own class, leaving the > rest of the functionality in the > BlackoilModel class. > > Further refactoring of this is faced with some nontrivial technical > challenges, centering on the > State classes. For example, many functions take a State object argument, > which (in the current approach) > are different between models (black-oil and b.o. with polymer for example). > Then that function > cannot be virtual (it would need to take the same argument). > > There seems to be two main alternatives: > > > 1. Applying the "curiously recurring template pattern" (it is indeed called > that!). This means > essentially using templates to get what we want, including the ability to > extend models, > reusing most functions and only modifying the few that we want to be > different. This will > all be checked at compile time, so illegal combinations of objects (such as > accidentally > using a State class without a certain member together with a model that > requires that member) > will not even compile. The drawback is that it involves a little more > boilerplate code than regular > inheritance: assuming that we want to an extensible black-oil model and a > polymer model that > extends it we'll have three classes, like this: > > template > class BlackoilModelBase > { > // This is where almost all the functions and data go. > }; > > class BlackoilModel : public BlackoilModelBase > { > // This contains only type definitions and a constructor. > }; > > class BlackoilPolymerModel: public BlackoilModelBase > { > // Contains type definitions and a constructor. > // Contains new functions, as well as new versions of functions from > BlackoilModelBase. > }; > > I do think that the boilerplate is manageable, and that people will get by > just fine by starting their own > extension by copying an existing extension. (The resulting copied code will > not be that many lines). > This pattern is a fairly common C++ idiom and is used a lot in Dune for > example. > > > 2. Using regular inheritance and a generic State class. This means we have a > single, flexible State > class that can be used for all purposes. It should function a bit like a > map>, > but not be implemented like that. This solution is most similar to how things > would be done in Matlab, > or other weakly typed languages: any usage errors would not be caught until > runtime. Each access > to the State will be a little more annoying than before, instead of > state.saturation() you would need to > call state.get("saturation") and it could possibly fail. > > In summary, none of these solutions are perfect, but both are workable. I > have discussed this with > Andreas, and we agreed to propose alternative 1 as the one we'd like to try > out first. I would like to > hear your opinions! > > Atgeirr The third option, which is somewhat similar to the first option (which by the way is much better than the broken common State class idiom) is to properly split things up into values and transactions and abandon the idea of a full class that is initialised, then called solve() upon. This means more flexibility in terms of replacing, plugging in and extending individual parts, as functions could be either arguments, static (aka hard coded) or compile-time arguments (hello templates), with the added benefit that you can replace sub components as containers with relative ease. Additionally, this actually opens for concurrency, something we should strive for in design considering the state we're in now. And if someone should still prefer having a class manage everything with init => solve, then this class would now be trivial to write. signature.asc Description: OpenPGP digital signature ___ Opm mailing list Opm@opm-project.org http://www.opm-project.org/mailman/listinfo/opm
Re: [OPM] Refactoring of the fully implicit solver class
22. mai 2015 kl. 13:46 skrev Joakim Hove : > Could code generation be a third alternative? You could think of templates as a form of structured code generation happening inside the C++ language. I think we want to keep this inside C++, since we are not talking about 100s of models, realistically up to a dozen I think. With so few models to generate the overhead to create a DSL, compiler or code generator is too large I think. Atgeirr ___ Opm mailing list Opm@opm-project.org http://www.opm-project.org/mailman/listinfo/opm
Re: [OPM] Refactoring of the fully implicit solver class
22. mai 2015 kl. 13:46 skrev Joakim Hove : > Could code generation be a third alternative? You could think of templates as a form of structured code generation happening inside the C++ language. I think we want to keep this inside C++, since we are not talking about 100s of models, realistically up to a dozen I think. With so few models to generate the overhead to create a DSL, compiler or code generator is too large I think. Atgeirr ___ Opm mailing list Opm@opm-project.org http://www.opm-project.org/mailman/listinfo/opm
Re: [OPM] Refactoring of the fully implicit solver class
Hi, On Friday 22 May 2015 11:46:11 Joakim Hove wrote: > There seems to be two main alternatives: > > Could code generation be a third alternative? not before you completely change the approach used by the current Opm simulators. If you go this route, you could also call it FeNICS ;) more seriously, I think that code generation is nice if everything works as intended, but if it doesn't, you'll have to deal with really hard to find and hard to debug bugs in the code generator. cheers Andreas -- If your text editor can defeat you at chess, it might be a bit overengineered. -- Jon Cruz, reflecting on the power of Emacs. signature.asc Description: This is a digitally signed message part. ___ Opm mailing list Opm@opm-project.org http://www.opm-project.org/mailman/listinfo/opm
Re: [OPM] Refactoring of the fully implicit solver class
There seems to be two main alternatives: Could code generation be a third alternative? J --- The information contained in this message may be CONFIDENTIAL and is intended for the addressee only. Any unauthorised use, dissemination of the information or copying of this message is prohibited. If you are not the addressee, please notify the sender immediately by return e-mail and delete this message. Thank you ___ Opm mailing list Opm@opm-project.org http://www.opm-project.org/mailman/listinfo/opm