-
Notifications
You must be signed in to change notification settings - Fork 26
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 PyBaMM IDAKLU solver, compiled from source #217
Add PyBaMM IDAKLU solver, compiled from source #217
Conversation
…to i12-unscented-kalman
Co-authored-by: Brady Planden <[email protected]>
Co-authored-by: Brady Planden <[email protected]>
Co-authored-by: Brady Planden <[email protected]>
Co-authored-by: Brady Planden <[email protected]>
Co-authored-by: Brady Planden <[email protected]>
Co-authored-by: Brady Planden <[email protected]>
Co-authored-by: Brady Planden <[email protected]>
…to i12-unscented-kalman
Co-authored-by: Agriya Khetarpal <[email protected]>
…ructural-parameters Add fitting functionality for geometric parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this PR @agriyakhetarpal. I've approved the workflow, it looks like we are running into a few issues builting from source.
Actually, building from source is working on the coverage tests, but it's trying to assume I have pushed d63fc76 which should fix this. I haven't added this step to the |
@BradyPlanden this should be ready for review now. Windows wheels for PyBaMM take ~20 minutes to build, and this can be reduced to ~4 minutes with caching for The Windows tests can be disabled by setting import sys
import pytest
ON_WINDOWS = sys.platform() == "win32"
@pytest.mark.skipif(ON_WINDOWS, reason="PyBaMM v24.1 is currently broken for Windows") can be added as necessary, which should resolve everything that has to be taken care of. The Windows wheels take a long time to build, I guess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to sort this @agriyakhetarpal. All looks good to me! I'll sort out the PyBOP failing tests in #176.
b78c6b5
into
pybop-team:160-update-basemodelsimulate-for-idaklu-output_variables
Till the time PyBaMM puts out a new release to fix the IDAKLU solver in its PyPI wheels, this PR shall help compile it from source in order for it to be used by PyBOP.
I have merged the latest changes from
develop
in this branch since #176 had not been updated in a while. This PR shall update the branch there upon getting merged, since this doesn't target thedevelop
branch.