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 PyBaMM IDAKLU solver, compiled from source #217

Conversation

agriyakhetarpal
Copy link
Member

@agriyakhetarpal agriyakhetarpal commented Feb 26, 2024

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 the develop branch.

martinjrobins and others added 30 commits December 18, 2023 16:58
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com>
@agriyakhetarpal agriyakhetarpal changed the title Compile PyBaMM IDAKLU solver, compiled from source Add PyBaMM IDAKLU solver, compiled from source Feb 26, 2024
Copy link
Member

@BradyPlanden BradyPlanden left a 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.

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Feb 26, 2024

Actually, building from source is working on the coverage tests, but it's trying to assume pybind11 is installed and then trying to run tests from its top-level directory and for PyBaMM. We have wanted to remove the git-clone for pybind11 in pybamm-team/PyBaMM#3560 but it has been stalled for many reasons.

I have pushed d63fc76 which should fix this. I haven't added this step to the build: job – I shall do that in a moment (edit: done in 91644c5)

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Feb 26, 2024

@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 vcpkg triplets – this is not worth using since they are broken as previously discussed. The Linux and macOS PyBaMM wheels are being compiled in ~3 minutes and work well – all of the test failures are coming from the PyBOP suite and you may take this up to fix up the test cases, either here, or merge this PR and then continue on #176, which is where these commits shall show up and then can be merged into the develop branch when ready.

The Windows tests can be disabled by setting if: false for the step in a further commit and a temporary pytest marker

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 pip install .[all] -vv (i.e., two levels of increased verbosity) should provide enough verbose logging.

@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review February 26, 2024 20:50
Copy link
Member

@BradyPlanden BradyPlanden left a 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.

@BradyPlanden BradyPlanden merged commit b78c6b5 into pybop-team:160-update-basemodelsimulate-for-idaklu-output_variables Feb 27, 2024
3 of 21 checks passed
@agriyakhetarpal agriyakhetarpal deleted the compile-pybamm-idaklu-solver-from-source branch February 27, 2024 18:45
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.

None yet

5 participants