Implemented dispatch for mimetic inner product selection #1190
Conversation
francoishamon
added
flag: requires rebaseline
francoishamon
changed the title
|
@klevzoff @rrsettgast I am done with the work that I wanted to do in this PR. This refactoring PR is not urgent at all and can wait, but if you have some time over the next weeks it is ready for review. |
There was a problem hiding this comment.
Looks great!
The only issue I have is with the inheritance of HybridMimeticDiscretization from FluxApproximationBase. Or rather the fact that FluxApproximationBase contains the stencils and related methods and HybridMimeticDiscretization has to inherit those and implement stubs. Cleaning up that hierarchy probably should be its own PR though.
| } | ||
| else | ||
| { | ||
| GEOSX_ERROR( "mimeticInnerProductDispatch() is not implemented for input of "<<typeid(input).name() ); |
There was a problem hiding this comment.
This should probably also use LvArray::system::demangleType
| } | ||
| else | ||
| { | ||
| GEOSX_ERROR( "mimeticInnerProductDispatch() is not implemented for input of "<<LvArray::system::demangleType( &input ) ); |
There was a problem hiding this comment.
No need for & in &input I think
|
Yes I agree with you about the class hierarchy in |
|
@klevzoff @rrsettgast I modified the |
| // 6) compute ( R^T R )^-1 | ||
| real64 temp[ NF ][ 3 ] = {{ 0 }}; | ||
| LvArray::tensorOps::copy< NF, 3 >( temp, cellToFaceMat ); // <<-- TODO: ask about this, we could have Rij_eq_AkiAkj instead | ||
| LvArray::tensorOps::Rij_eq_AkiBkj< 3, 3, NF >( work_dimByDim, | ||
| temp, | ||
| cellToFaceMat ); |
There was a problem hiding this comment.
@corbett5 In this function, I need to compute R^T R, where R is a rectangular matrix of size NF x 3. Currently I create a temporary matrix to be able to use LvArray::tensorOps::Rij_eq_AkiBkj. Is there already something in LvArray to do the same thing without the temporary matrix? If not, would it make sense to add something like Rij_eq_AkiAkj?
There was a problem hiding this comment.
I don't think there is currently anything to do this since the restrict keyword prevents the usage of AkiBkj. But yes given this restriction it would absolutely make sense to add. @rrsettgast and @CusiniM have a PR to do the same thing for other functions.
There was a problem hiding this comment.
@francoishamon We can quickly add it to that PR if you want.
There was a problem hiding this comment.
Yes that would be great (if you don't mind). Do you want to do it? Or should I do it and commit to your PR? Thanks
There was a problem hiding this comment.
I have done it. I just need to add a unit test for it.
francoishamon
added
ci: run CUDA builds
This PR modifies the hybrid/mimetic FVM solver to allow the user to select a mimetic inner product from the input file in the
<FiniteVolume>block of the XML file.To do that I adapted some constructs used in
FiniteElementDiscretization,FiniteElementDispatch, and the folderelementFormulations. Each mimetic inner product has its own class with a static member function that implements the calculation of the transmissibility matrix. To select the inner product inSinglePhaseHybridFVM.cpp, I use a dispatch just before launching theFluxKernel(now templated on the inner product type).Most of the changes of the PR come from the fact that, after the addition of the new template, I was not able to compile with explicit instantiation of the functions in
SinglePhaseHybridFVMKernels.cppanymore (maybe there is a workaround but I could not find it). So I moved all the function implementations to the .hpp.Related to https://github.com/GEOSX/integratedTests/pull/107.