GEOS icon indicating copy to clipboard operation
GEOS copied to clipboard

Corbett/kernel specs

Open corbett5 opened this issue 1 year ago • 1 comments

TODO: If the json file is modified CMake doesn't run again.

Not a fan of having to duplicate the function declarations again in the .cpp.template file. Many of the functions have very complicated declarations and the errors obtained from mistakes are sure to cause headaches. For me this was in physicsSolvers/fluidFlow/CompositionalMultiphaseHybridFVMKernels_impl.hpp. One alternative is to define explicit instantiation macros in this header so at least you don't have to update two different files. You'd still have to list out the function arguments twice unless we resorted to some more intrusive macros.

corbett5 avatar Aug 07 '24 21:08 corbett5

@rrsettgast @wrtobin I think you guys should give this a look over. I commented a few of the pain points. It passes most of the integrated tests on ruby (except a few FPE runtime errors and restart checks I think are unrelated to this PR) and I have been unable to run them on Lassen.

All in all the compilation is still extremely slow. It takes almost two hours to compile on Lassen, most of which is spent linking.

corbett5 avatar Oct 18 '24 16:10 corbett5

Codecov Report

:x: Patch coverage is 84.42623% with 76 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 56.89%. Comparing base (c50f717) to head (b4c106e). :warning: Report is 50 commits behind head on develop.

Files with missing lines Patch % Lines
...nents/finiteElement/kernelInterface/KernelBase.hpp 0.00% 32 Missing :warning:
...l/CompositionalMultiphaseHybridFVMKernels_impl.hpp 96.69% 14 Missing :warning:
...hanics/contact/SolidMechanicsEmbeddedFractures.cpp 0.00% 9 Missing :warning:
src/coreComponents/mesh/ElementRegionManager.hpp 0.00% 5 Missing :warning:
...sics/SinglePhasePoromechanicsEmbeddedFractures.hpp 0.00% 4 Missing :warning:
...ers/solidMechanics/SolidMechanicsLagrangianFEM.cpp 0.00% 4 Missing :warning:
...hysicsSolvers/multiphysics/PoromechanicsSolver.hpp 0.00% 3 Missing :warning:
...ers/solidMechanics/SolidMechanicsLagrangianFEM.hpp 0.00% 2 Missing :warning:
src/coreComponents/common/TypeDispatch.hpp 0.00% 1 Missing :warning:
src/coreComponents/mesh/FaceElementSubRegion.cpp 0.00% 1 Missing :warning:
... and 1 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3273      +/-   ##
===========================================
- Coverage    56.91%   56.89%   -0.02%     
===========================================
  Files         1228     1228              
  Lines       106553   106579      +26     
===========================================
  Hits         60642    60642              
- Misses       45911    45937      +26     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar May 02 '25 06:05 codecov[bot]

Thanks.

I would start by merging it as it is now. In the future, I would add the option to specify via command line a json file and store a couple in the repository with minimal configurations (only elasticity and hexahedra). We may also want to break it down into multiple files.

To me it would make sense if the full specifications for each solver lived in a json file adjacent to the solver itself, but we'd certainly want to keep the ability for a user to specify a single override file.

corbett5 avatar May 02 '25 23:05 corbett5