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

Set solver tolerances in ElectrodeSOH classes and methods #3688

Closed
rtimms opened this issue Jan 4, 2024 · 0 comments · Fixed by #3714
Closed

Set solver tolerances in ElectrodeSOH classes and methods #3688

rtimms opened this issue Jan 4, 2024 · 0 comments · Fixed by #3714
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: medium To be resolved if time allows

Comments

@rtimms
Copy link
Contributor

rtimms commented Jan 4, 2024

Calls to get_initial_stoichiometries end up using an algebraic solver with a fixed tolerance of 1e-6.

Sometimes this throws an error if the solver tolerance is not met, but users may want to accept a solution with a larger tolerance. It would be good to be able to pass the solver tolerance as an optional argument to get_initial_stoichiometries and other places where it is called, e.g. ParameterValues.set_initial_stoichiometries.

@rtimms rtimms added difficulty: easy A good issue for someone new. Can be done in a few hours priority: medium To be resolved if time allows labels Jan 4, 2024
kratman added a commit that referenced this issue Jan 24, 2024
#3714)

* Refactor get_initial_stoichiometries with custom and default tolerance

This commit enhances the `get_initial_stoichiometries` method by introducing a new parameter, `solver_tolerance`. The primary change allows users to specify a custom tolerance level for the solver.

The method now includes a default solver tolerance set at 1e-6, maintaining a balance between precision and computational efficiency for general use cases. Additionally, users can adjust this tolerance level as needed, catering to specific requirements of different battery modeling scenarios.

Resolves: #3688

* style: pre-commit fixes

* Refactor argument name from 'solver_tolerance' to 'tol' for consistency with AlgebraicSolver class

* Update CHANGELOG.md

* Fix CHANGELOG.md

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
js1tr3 pushed a commit to js1tr3/PyBaMM that referenced this issue Aug 12, 2024
pybamm-team#3714)

* Refactor get_initial_stoichiometries with custom and default tolerance

This commit enhances the `get_initial_stoichiometries` method by introducing a new parameter, `solver_tolerance`. The primary change allows users to specify a custom tolerance level for the solver.

The method now includes a default solver tolerance set at 1e-6, maintaining a balance between precision and computational efficiency for general use cases. Additionally, users can adjust this tolerance level as needed, catering to specific requirements of different battery modeling scenarios.

Resolves: pybamm-team#3688

* style: pre-commit fixes

* Refactor argument name from 'solver_tolerance' to 'tol' for consistency with AlgebraicSolver class

* Update CHANGELOG.md

* Fix CHANGELOG.md

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: medium To be resolved if time allows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant