Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add forcing function to general Rosenbrock solver #74

Merged
merged 5 commits into from
Jun 1, 2023

Conversation

mattldawson
Copy link
Collaborator

@mattldawson mattldawson commented Jun 1, 2023

Adds a general forcing function to the Rosenbrock solver.

The forcing calculations were added to a new ProcessSet class because they are specific to gas-phase chemical systems and not really to general ODE solvers, but I'm not sure if this is the best solution.

Minor changes:

  • changed the Solve() function on the Rosenbrock solver to only take the time step info and a State object.
  • added const Matrix functions

closes #63

include/micm/process/process_set.hpp Outdated Show resolved Hide resolved

inline ProcessSet::ProcessSet(const std::vector<Process>& processes, const State& state)
: number_of_reactants_(
[&]() -> std::vector<std::size_t>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the data member are not const, it might be more readable to move these into the body of the constructor. We would no longer need the lambda functions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to constructor body

concentrations_(parameters.number_of_grid_cells_, parameters.number_of_state_variables_, 0.0 ),
custom_rate_parameters_(parameters.number_of_grid_cells_, parameters.number_of_custom_parameters_, 0.0 ),
rate_constants_(parameters.number_of_grid_cells_, parameters.number_of_rate_constants_, 0.0 )
variable_map_(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too, I think we should move the contents of the lambda function into the body of the constructor so that it's a little easier to read

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to constructor body

@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2023

Codecov Report

Patch coverage: 98.13% and project coverage change: +15.72 🎉

Comparison is base (41c0359) 75.49% compared to head (03e8058) 91.22%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main      #74       +/-   ##
===========================================
+ Coverage   75.49%   91.22%   +15.72%     
===========================================
  Files          15       16        +1     
  Lines         755      832       +77     
===========================================
+ Hits          570      759      +189     
+ Misses        185       73      -112     
Impacted Files Coverage Δ
include/micm/system/system.hpp 90.90% <80.00%> (-1.40%) ⬇️
include/micm/process/process.hpp 94.44% <100.00%> (ø)
include/micm/process/process_set.hpp 100.00% <100.00%> (ø)
include/micm/solver/chapman_ode_solver.hpp 93.00% <100.00%> (+0.06%) ⬆️
include/micm/solver/rosenbrock.hpp 85.65% <100.00%> (+52.47%) ⬆️
include/micm/solver/state.hpp 100.00% <100.00%> (ø)
include/micm/util/matrix.hpp 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@K20shores K20shores merged commit a738295 into main Jun 1, 2023
@K20shores K20shores deleted the develop-63-forcing branch June 1, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add general p_force function to RosenbrockODESolver
4 participants