search

NCAR / micm

A model-independent chemistry module for atmosphere models
https://ncar.github.io/micm/
Apache License 2.0
6 stars 4 forks source link

Apply solver builder to Rosenbrock #560

Closed mattldawson closed 3 weeks ago

mattldawson commented 4 weeks ago

Modifies the Rosenbrock solver to be creatable using the SolverBuilder class. This work coincidentally also removed the remaining template template parameters. Also, moves the rate constant calculation step out of the solver function, and modifies the GH Action for coverage to run on pushes to main, so that CodeCov diffs stay up-to-date

closes #529 closes #538 closes #539 closes #496 closes musica/134

codecov-commenter commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 89.63415% with 17 lines in your changes missing coverage. Please review.

Project coverage is 92.70%. Comparing base (555be4e) to head (2ce0237). Report is 12 commits behind head on main.

Files Patch % Lines
include/micm/solver/solver_result.hpp 27.27% 8 Missing :warning:
include/micm/solver/solver_builder.inl 90.00% 4 Missing :warning:
include/micm/solver/jit_rosenbrock.hpp 75.00% 3 Missing :warning:
include/micm/solver/backward_euler.inl 90.00% 1 Missing :warning:
include/micm/util/sparse_matrix.hpp 90.90% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #560 +/- ## ========================================== + Coverage 92.25% 92.70% +0.44% ========================================== Files 45 48 +3 Lines 3407 3480 +73 ========================================== + Hits 3143 3226 +83 + Misses 264 254 -10 ```

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

mattldawson commented 3 weeks ago

@mattldawson can you give a brief flowchart or introduction about the general steps to use the new solverbuilder class to build a solver for CPU/GPU? Thanks.

A CPU solver is built in the README example here: https://github.com/NCAR/micm/blob/6b973a054282386cf03a86fec916ec07b95cfe00/test/tutorial/test_README_example.cpp#L34-L37

I don't think we have a full CUDA Rosenbrock example yet in the tutorial, but I think it would follow logic similar to the JIT solver: https://github.com/NCAR/micm/blob/6b973a054282386cf03a86fec916ec07b95cfe00/test/tutorial/test_jit_tutorial.cpp#L104-L108

mattldawson commented 3 weeks ago

Thanks @mattldawson for working on this giant PR. It is lots of work!

It seems that two tests on the windows platform fail the CI workflow somehow and the error message indicates an internal compiler error. Are they just old versions and should be removed? Or do they indicate a potential code bug somewhere?

They just mean there is a bug in the compiler code. I don't think there is a way to infer anything beyond that. I will create a separate issue to look into that.

sjsprecious commented 3 weeks ago

Thanks @mattldawson for working on this giant PR. It is lots of work! It seems that two tests on the windows platform fail the CI workflow somehow and the error message indicates an internal compiler error. Are they just old versions and should be removed? Or do they indicate a potential code bug somewhere?

They just mean there is a bug in the compiler code. I don't think there is a way to infer anything beyond that. I will create a separate issue to look into that.

Thanks @mattldawson . That sounds good.

K20shores commented 3 weeks ago

Does this also close #496 and #499?

mattldawson commented 3 weeks ago

Does this also close #496 and #499?

I think it closes #496, but I didn't include the analytical tests for Backward Euler because they were failing (probably needs different solver tolerances, or lower expectations as @sjsprecious mentioned)