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

i3558 improve wheel repair + add minimal IDAKLU test for built wheels #3569

Merged
merged 28 commits into from
Jan 6, 2024

Conversation

agriyakhetarpal
Copy link
Member

@agriyakhetarpal agriyakhetarpal commented Nov 27, 2023

Description

This PR sets the runtime search path for the idaklu target via CMake to the location of the CasADi libraries so that they can be picked up by auditwheel and delocate at the time of repairing the wheel. This path was strangely empty in prior versions and therefore a script was used to fix the RPATH for macOS. A small test has been added to each wheel job to check the import of the IDAKLU extension module, which, upon being failed will mark the workflow as such too.

Tip

I have triggered a workflow run to build the wheels under these changes, it can be accessed from here: https://github.com/agriyakhetarpal/PyBaMM/actions/runs/7009012889

Closes #3558

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

  • Bug fix (non-breaking change which fixes an issue)

Key checklist:

  • No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
  • All tests pass: $ python run-tests.py --all (or $ nox -s tests)
  • The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1d2bf31) 99.59% compared to head (632bcec) 99.59%.
Report is 20 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3569      +/-   ##
===========================================
- Coverage    99.59%   99.59%   -0.01%     
===========================================
  Files          258      258              
  Lines        20798    20796       -2     
===========================================
- Hits         20713    20711       -2     
  Misses          85       85              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kratman
Copy link
Contributor

kratman commented Nov 27, 2023

Thanks, I will try to take a look at this tonight or tomorrow

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

Thanks, @agriyakhetarpal! A couple of comments below -

.github/workflows/publish_pypi.yml Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

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

this looks great, thanks @agriyakhetarpal. @pipliggins also had problems with the casadi rpath on mac, @pipliggins does this fix this?

@pipliggins
Copy link
Contributor

Sadly not @martinjrobins - @agriyakhetarpal I have the same issue as that discussed in #3100. We've tracked down the issue to (potentially) being caused by me having an older version of casadi (3.6.0 rather than 3.6.4) elsewhere in my system, and idaklu is binding to that rather than the version pulled during the pybamm install. The changes you've made here don't appear to have affected this behaviour.

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Nov 30, 2023

I had the same thing in mind too very recently when trying to find a solution for #3100. I notice that the version bounds for casadi as a build-time and a run-time dependency are different (3.6.0 vs 3.6.3); it is possible and very reasonable that the IDAKLU target might have linked to an older version, say, 3.5.5 coming from our vcpkg registry when we had started supporting 3.6 which caused the serialisation error in #3100 (not sure if we had updated the version in the registry in time but that has been the only complaint we received with 23.5 so there might be more to it)

Hi @pipliggins – if it's possible for you, could you create a fresh virtual environment, install casadi 3.6.4 and run the editable installation again just so it tries to link to the one inside your virtual environment? Edit: testing on my end with two instances: with and without casadi pre-installed into the venv, importlib found the correct path for me. It found the one in the venv if I already had it and used the one installed by the pip build tracker if I didn't

@pipliggins
Copy link
Contributor

@agriyakhetarpal no joy. @jsbrittain previously suggested something similar - "adding 'casadi' to line 121 of noxfile.py (set_dev function): session.install("virtualenv", "cmake", "casadi")," to try and fix it via nox which also didn't seem to work.

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Nov 30, 2023

Yes, doing it via the nox session would have meant the same thing. Is there a trace you can share from the IDAKLU compilation procedure? Maybe that can reveal further details, since we don't have the logs from six months back when 23.5 was released...

Also, does invoking a clean pybamm-requires session help?

@pipliggins
Copy link
Contributor

I can get this from the editable install:

SuiteSparse found in /Users/pybamm_user/.local/include: /Users/pybamm_user/.local/lib/libsuitesparseconfig.dylib
-- Configuring done (3.7s)
-- Generating done (0.0s)
-- Build files have been written to: /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmpi_lyi4o3.build-temp
[ 9%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/casadi_functions.cpp.o
[ 18%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/casadi_solver.cpp.o
[ 27%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/CasadiSolver.cpp.o
[ 36%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/CasadiSolverOpenMP.cpp.o
[ 45%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/CasadiSolverOpenMP_solvers.cpp.o
[ 54%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/casadi_sundials_functions.cpp.o
[ 63%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/python.cpp.o
[ 72%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/solution.cpp.o
[ 81%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/options.cpp.o
[ 90%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu.cpp.o
In file included from /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu.cpp:1:
In file included from /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu/casadi_solver.hpp:4:
In file included from /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu/CasadiSolver.hpp:4:
In file included from /usr/local/include/casadi/casadi.hpp:29:
In file included from /usr/local/include/casadi/core/core.hpp:30:
In file included from /usr/local/include/casadi/core/sx_elem.hpp:30:
In file included from /usr/local/include/casadi/core/printable.hpp:29:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/iostream:43:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/ios:221:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__locale:18:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/mutex:191:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__memory/shared_ptr.h:31:
/Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__memory/unique_ptr.h:65:5: warning: delete called on 'CasadiSolver' that is abstract but has non-virtual destructor [-Wdelete-abstract-non-virtual-dtor]
delete __ptr;
^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__memory/unique_ptr.h:297:7: note: in instantiation of member function 'std::default_delete::operator()' requested here
_ptr.second()(__tmp);
^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/memory/unique_ptr.h:263:75: note: in instantiation of member function 'std::unique_ptr::reset' requested here
LIBCPP_INLINE_VISIBILITY LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
^
/Users/pybamm_user/Documents/PyBaMM/pybind11/include/pybind11/pybind11.h:1923:40: note: in instantiation of member function 'std::unique_ptr::~unique_ptr' requested here
v_h.holder<holder_type>().~holder_type();
^
/Users/pybamm_user/Documents/PyBaMM/pybind11/include/pybind11/pybind11.h:1585:26: note: in instantiation of member function 'pybind11::class
::dealloc' requested here
record.dealloc = dealloc;
^
/Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu.cpp:50:3: note: in instantiation of function template specialization 'pybind11::class
::class
<>' requested here
py::class
(m, "CasadiSolver")
^
1 warning generated.
[100%] Linking CXX shared module idaklu.cpython-311-darwin.so
[100%] Built target idaklu
---------- Running CMake for IDAKLU solver ----------------------------------------
---------- Building IDAKLU module ----------------------------------------
copying /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmpi_lyi4o3.build-temp/idaklu.cpython-311-darwin.so -> /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers

Is there a way to isolate the IDAKLU install more to get a better trace?

I've done a clean install of pybamm (deleted the entire folder and started the developer install process from scratch) for this

@agriyakhetarpal
Copy link
Member Author

Is there a way to isolate the IDAKLU install more to get a better trace?

I would suggest that doing pip install -e . -vvv, i.e., with the highest level of verbosity would help too.

Also, I think this output is truncated and starts from the part where SuiteSparse is linked, could you share it from the part where CMake tries to find casadi or perhaps the entire CMake output?

@pipliggins
Copy link
Contributor

This is with -vvv

Building wheels for collected packages: pybamm
Created temporary directory: /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv
Destination directory: /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv
Running command Building editable for pybamm (pyproject.toml)
cmake version 3.27.7

CMake suite maintained and supported by Kitware (kitware.com/cmake).
running editable_wheel
creating /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info
writing /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/PKG-INFO
writing dependency_links to /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/dependency_links.txt
writing entry points to /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/entry_points.txt
writing requirements to /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/requires.txt
writing top-level names to /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/top_level.txt
writing manifest file '/private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/SOURCES.txt'
reading manifest file '/private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no previously-included files found matching 'CHANGELOG.md'
warning: no previously-included files found matching 'CODE-OF-CONDUCT.md'
warning: no previously-included files found matching 'CONTRIBUTING.md'
warning: no previously-included files found matching 'CMakeLists.txt'
warning: no previously-included files matching 'pycache' found anywhere in distribution
warning: no previously-included files matching '*.py[cod]' found anywhere in distribution
warning: no previously-included files matching '.venv' found anywhere in distribution
adding license file 'LICENSE.txt'
writing manifest file '/private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm.egg-info/SOURCES.txt'
creating '/private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm-23.9.dist-info'
creating /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs_/pybamm-23.9.dist-info/WHEEL
running build_py
running build_ext
-- The C compiler identification is AppleClang 15.0.0.15000040
-- The CXX compiler identification is AppleClang 15.0.0.15000040
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- pybind11 v2.12.0 dev1
CMake Warning (dev) at pybind11/tools/FindPythonLibsNew.cmake:101 (message):
Policy CMP0148 is not set: The FindPythonInterp and FindPythonLibs modules
are removed. Run "cmake --help-policy CMP0148" for policy details. Use
the cmake_policy command to set the policy and suppress this warning, or
preferably upgrade to using FindPython, either by calling it explicitly
before pybind11, or by setting PYBIND11_FINDPYTHON ON before pybind11.
Call Stack (most recent call first):
pybind11/tools/pybind11Tools.cmake:50 (find_package)
pybind11/tools/pybind11Common.cmake:188 (include)
pybind11/CMakeLists.txt:222 (include)
This warning is for project developers. Use -Wno-dev to suppress it.

-- Found PythonInterp: /Users/pybamm_user/Documents/PyBaMM/venv_test2/bin/python3.11 (found suitable version "3.11.6", minimum required is "3.6")
-- Found PythonLibs: /opt/homebrew/opt/python@3.11/Frameworks/Python.framework/Versions/3.11/lib/libpython3.11.dylib
-- Performing Test HAS_FLTO_THIN
-- Performing Test HAS_FLTO_THIN - Failed
-- Performing Test HAS_FLTO
-- Performing Test HAS_FLTO - Success
Found python casadi path: /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-build-env-2cwfxc9y/overlay/lib/python3.11/site-packages/casadi
Trying to link against python casadi package
-- Found SUNDIALS: /Users/pybamm_user/.local/include
SUNDIALS found in /Users/pybamm_user/.local/include: /Users/pybamm_user/.local/lib/libsundials_idas.dylib;/Users/pybamm_user/.local/lib/libsundials_sunlinsolklu.dylib;/Users/pybamm_user/.local/lib/libsundials_sunlinsoldense.dylib;/Users/pybamm_user/.local/lib/libsundials_sunlinsolspbcgs.dylib;/Users/pybamm_user/.local/lib/libsundials_sunlinsollapackdense.dylib;/Users/pybamm_user/.local/lib/libsundials_sunmatrixsparse.dylib;/Users/pybamm_user/.local/lib/libsundials_nvecserial.dylib;/Users/pybamm_user/.local/lib/libsundials_nvecopenmp.dylib
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found SuiteSparse: TRUE
SuiteSparse found in /Users/pybamm_user/.local/include: /Users/pybamm_user/.local/lib/libsuitesparseconfig.dylib
-- Configuring done (2.8s)
-- Generating done (0.0s)
-- Build files have been written to: /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp
[ 9%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/casadi_functions.cpp.o
[ 18%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/casadi_solver.cpp.o
[ 27%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/CasadiSolver.cpp.o
[ 36%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/CasadiSolverOpenMP.cpp.o
[ 45%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/CasadiSolverOpenMP_solvers.cpp.o
[ 54%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/casadi_sundials_functions.cpp.o
[ 63%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/python.cpp.o
[ 72%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/solution.cpp.o
[ 81%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu/options.cpp.o
[ 90%] Building CXX object CMakeFiles/idaklu.dir/pybamm/solvers/c_solvers/idaklu.cpp.o
In file included from /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu.cpp:1:
In file included from /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu/casadi_solver.hpp:4:
In file included from /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu/CasadiSolver.hpp:4:
In file included from /usr/local/include/casadi/casadi.hpp:29:
In file included from /usr/local/include/casadi/core/core.hpp:30:
In file included from /usr/local/include/casadi/core/sx_elem.hpp:30:
In file included from /usr/local/include/casadi/core/printable.hpp:29:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/iostream:43:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/ios:221:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__locale:18:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/mutex:191:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__memory/shared_ptr.h:31:
/Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__memory/unique_ptr.h:65:5: warning: delete called on 'CasadiSolver' that is abstract but has non-virtual destructor [-Wdelete-abstract-non-virtual-dtor]
delete __ptr;
^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/__memory/unique_ptr.h:297:7: note: in instantiation of member function 'std::default_delete::operator()' requested here
_ptr.second()(__tmp);
^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/c++/v1/memory/unique_ptr.h:263:75: note: in instantiation of member function 'std::unique_ptr::reset' requested here
LIBCPP_INLINE_VISIBILITY LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
^
/Users/pybamm_user/Documents/PyBaMM/pybind11/include/pybind11/pybind11.h:1923:40: note: in instantiation of member function 'std::unique_ptr::~unique_ptr' requested here
v_h.holder<holder_type>().~holder_type();
^
/Users/pybamm_user/Documents/PyBaMM/pybind11/include/pybind11/pybind11.h:1585:26: note: in instantiation of member function 'pybind11::class
::dealloc' requested here
record.dealloc = dealloc;
^
/Users/pybamm_user/Documents/PyBaMM/pybamm/solvers/c_solvers/idaklu.cpp:50:3: note: in instantiation of function template specialization 'pybind11::class
::class
<>' requested here
py::class
(m, "CasadiSolver")
^
1 warning generated.
[100%] Linking CXX shared module idaklu.cpython-311-darwin.so
[100%] Built target idaklu
---------- Running CMake for IDAKLU solver ----------------------------------------
---------- Building IDAKLU module ----------------------------------------
copying /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/idaklu.cpython-311-darwin.so -> /Users/pybamm_user/Documents/PyBaMM/pybamm/solvers
running egg_info
creating /var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info
writing /var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/PKG-INFO
writing dependency_links to /var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/dependency_links.txt
writing entry points to /var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/entry_points.txt
writing requirements to /var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/requires.txt
writing top-level names to /var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/top_level.txt
writing manifest file '/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/SOURCES.txt'
reading manifest file '/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no previously-included files found matching 'CHANGELOG.md'
warning: no previously-included files found matching 'CODE-OF-CONDUCT.md'
warning: no previously-included files found matching 'CONTRIBUTING.md'
warning: no previously-included files found matching 'CMakeLists.txt'
warning: no previously-included files matching 'pycache' found anywhere in distribution
warning: no previously-included files matching '*.py[cod]' found anywhere in distribution
warning: no previously-included files matching '.venv' found anywhere in distribution
adding license file 'LICENSE.txt'
writing manifest file '/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmp4w8bylm7.build-temp/pybamm.egg-info/SOURCES.txt'
Editable install will be performed using a meta path finder.

Options like package-data, include/exclude-package-data or
packages.find.exclude/include may have no effect.

adding '_editable___pybamm_23_9_finder.py'
adding 'editable.pybamm-23.9.pth'
creating '/private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-wheel-rcwok5fv/.tmp-0x_9ahs
/pybamm-23.9-0.editable-cp311-cp311-macosx_14_0_arm64.whl' and adding '/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/tmprq_0xn3upybamm-23.9-0.editable-cp311-cp311-macosx_14_0_arm64.whl' to it
adding 'pybamm-23.9.dist-info/LICENSE.txt'
adding 'pybamm-23.9.dist-info/METADATA'
adding 'pybamm-23.9.dist-info/WHEEL'
adding 'pybamm-23.9.dist-info/entry_points.txt'
adding 'pybamm-23.9.dist-info/top_level.txt'
adding 'pybamm-23.9.dist-info/RECORD'
/private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-build-env-2cwfxc9y/overlay/lib/python3.11/site-packages/setuptools/command/editable_wheel.py:347: InformationOnly: Editable installation.
!!

      ********************************************************************************
      Please be careful with folders in your working directory with the same
      name as your package as they may take precedence during imports.
      ********************************************************************************

!!
with strategy, WheelFile(wheel_path, "w") as wheel_obj:
Building editable for pybamm (pyproject.toml) ... done
Created wheel for pybamm: filename=pybamm-23.9-0.editable-cp311-cp311-macosx_14_0_arm64.whl size=13661 sha256=75e0df876c1ebdbc94b0caadcf83f9d895bb4b1bcbc578df5a6016bd020a6b04
Stored in directory: /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-ephem-wheel-cache-ba4m37_o/wheels/8b/f1/55/0c1a090ee8f018a245341bb4c6e27d676ab8d8afc089b45af9
Successfully built pybamm

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Dec 1, 2023

Okay, from the above logs

Found python casadi path: /private/var/folders/rs/lr0lp0yn56z2bfv97q0cj_q00000gp/T/pip-build-env-2cwfxc9y/overlay/lib/python3.11/site-packages/casadi

It indeed links to the correct casadi installation fetched from the build-system dependencies table instead of a different casadi installed in a different location. Edit: there is an update, please see the conversation on #3100!

CMake showed a warning about these arguments not being used during the compilation of the project.
SuiteSparse dynamic libraries were being repeated in the list of paths without this configuration. Setting build rpaths for AMD, COLAMD, BTF, and KLU ensures that they do not reference the SuiteSparse config in the build folder but the one that is installed into the install prefix.
@agriyakhetarpal
Copy link
Member Author

I have now gone further in this PR and fixed the RPATH via CMake for the compilation of the relevant SuiteSparse components, through which we can now get rid of fix_suitesparse_rpath_mac.sh. I built wheels for macOS in a workflow run on my fork just to make sure everything works, and it does: https://github.com/agriyakhetarpal/PyBaMM/actions/runs/7212519109. I think this should be ready to merge now (I can't do anything about Codacy in the scenario of these changes)

@agriyakhetarpal
Copy link
Member Author

The CMake parallel builds in a04fce6 have reduced the pybamm-requires compilation time to ~37 seconds on Linux and under two minutes on macOS. This is at least a 2x gain on builds where the cache is not hit. It should be possible to optimise this further, will open a separate issue for tracking the improvements to the script as discussed with Martin and include some other specifics there.

Copy link
Member

@arjxn-py arjxn-py left a comment

Choose a reason for hiding this comment

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

@agriyakhetarpal, you may want to merge to develop to address the conflicts

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

This looks great, thanks @agriyakhetarpal!

Copy link
Member

@arjxn-py arjxn-py left a comment

Choose a reason for hiding this comment

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

Thanks @agriyakhetarpal, looks very good.

pybamm/install_odes.py Show resolved Hide resolved
@agriyakhetarpal agriyakhetarpal force-pushed the i3558-repair-wheel branch 2 times, most recently from 7c82e87 to aa87d69 Compare January 6, 2024 10:26
@Saransh-cpp Saransh-cpp merged commit 051e233 into pybamm-team:develop Jan 6, 2024
38 of 41 checks passed
@agriyakhetarpal agriyakhetarpal deleted the i3558-repair-wheel branch January 6, 2024 20:06
@agriyakhetarpal agriyakhetarpal mentioned this pull request Jan 24, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants