search

pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
1.07k stars 528 forks source link

Revisit linkage with Python `casadi` for Windows wheels and move away from `pybamm-team/casadi-vcpkg-registry` #3603

Open agriyakhetarpal opened 9 months ago

agriyakhetarpal commented 9 months ago

Please read through #3100 and #3193 for background discussion. This proposition is about getting back to linking against the Python-based CasADi distribution from PyPI when building the IDAKLU extension for the Windows wheels. We currently use a custom vcpkg registry at https://github.com/pybamm-team/casadi-vcpkg-registry/ for this reason, but further releases mean that the versions for casadi at build-time and that at run-time can sometimes run out of sync with each other due to lack of periodic updates to the repository, and might cause issues with the import of the IDAKLUSolver after importing PyBaMM such as the one introduced in the PyPI wheels for certain users where a serialisation error was introduced (#3193).

This would serve two purposes:

  1. Provide a more maintainable solution for building the Windows wheels, since the only vcpkg registry we would need to interface would be the one for SUNDIALS – which is needed because we require it to be built and distributed with KLU
  2. Use the same version for build-time and run-time dependencies which ensures a stable binary.

I have tried building the Windows wheels and used casadi==3.6.4 from PyPI for the linkage in a workflow run for my fork, but it resulted in an error where the libcasadi.lib file was requested but just casadi.lib exists in the casadi/ folder, this might be a mismatch from their end or some error in our configuration (see https://github.com/pybamm-team/PyBaMM/issues/3100#issuecomment-1845352546)

agriyakhetarpal commented 8 months ago

I have been reading about this a bit and one way is to set a filesystem registry instead of a Git registry. I haven't tried this locally yet. Another thing we can look into for the time being is to move both Git registries into one registry (one single repository) since even that would ease things up by a bit.

jbuonagurio commented 6 months ago

Hi, I just submitted PR https://github.com/microsoft/vcpkg/pull/36885 which adds klu feature support to the vcpkg sundials port (as well as all of the other linear solvers + backends available in vcpkg). Hopefully I can get it merged without too much trouble.