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

Fix for new scipy #105

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix for new scipy #105

wants to merge 8 commits into from

Conversation

cmhamel
Copy link
Collaborator

@cmhamel cmhamel commented Jan 7, 2025

Add a scipy version restriction after encountering terrible errors in a plato build.

The culprit was a method call somewhere in Objective.py with an error similar to

  what():  TypeError: primal and tangent arguments to jax.jvp do not match; dtypes must be equal, or in case of int/bool primal dtype the tangent dtype must be float0.Got primal dtype float64 and so expected tangent dtype float64, but got tangent dtype int8 instead.
__notes__ (len=1):

This error was encountered on scipy 1.15.0 and no issues were encountered on 1.14.1. Something in scipy changed that jax is not tracing properly.

Add a scipy version restriction after encountering terrible errors in a plato build. 

The culprit was a method call somewhere in ```Objective.py``` with an error similar to 

```terminate called after throwing an instance of 'pybind11::error_already_set'
  what():  TypeError: primal and tangent arguments to jax.jvp do not match; dtypes must be equal, or in case of int/bool primal dtype the tangent dtype must be float0.Got primal dtype float64 and so expected tangent dtype float64, but got tangent dtype int8 instead.
__notes__ (len=1):```
Copy link
Collaborator

@tupek2 tupek2 left a comment

Choose a reason for hiding this comment

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

Interesting. I had this error. I can fix it with this change on line~17 in WarmStart.py:
op = lambda v: objective.hessian_vec(x, np.array(v, dtype=np.float64))
This let me upgrade to newer jax versions. Apparently scipy these days will pass integer vectors to iterative solvers sometimes?

fixing to an upper bound since 1.14.1 doesn't exist for python 3.9
@cmhamel
Copy link
Collaborator Author

cmhamel commented Jan 7, 2025

Interesting. I had this error. I can fix it with this change on line~17 in WarmStart.py: op = lambda v: objective.hessian_vec(x, np.array(v, dtype=np.float64)) This let me upgrade to newer jax versions. Apparently scipy these days will pass integer vectors to iterative solvers sometimes?

Sounds good to me Mike. Glad you dug a little deeper than I did. I was happy just to get things working again.

Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.18%. Comparing base (04b7883) to head (ed7c413).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #105   +/-   ##
=======================================
  Coverage   77.18%   77.18%           
=======================================
  Files          62       62           
  Lines        5225     5225           
=======================================
  Hits         4033     4033           
  Misses       1192     1192           

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

@ralberd ralberd changed the title Update setup.py Fix for new scipy Jan 8, 2025
@btalamini
Copy link
Collaborator

@ralberd , hod on, don't merge - Mike has a more fundamental fix. See comments above.

@cmhamel
Copy link
Collaborator Author

cmhamel commented Jan 8, 2025

@ralberd , hod on, don't merge - Mike has a more fundamental fix. See comments above.

I'm adding Mike's fix @btalamini

@tupek2
Copy link
Collaborator

tupek2 commented Jan 8, 2025

Yeah, my suggested fix is in there. Hopefully that addresses it, but its sort of odd still.

@cmhamel
Copy link
Collaborator Author

cmhamel commented Jan 8, 2025

Now something is up with metis...

@ralberd
Copy link
Contributor

ralberd commented Jan 9, 2025

In light of the issues Craig is running into, I'm going to open a separate MR that just fixes the scipy version to 1.14.1 since that works. We can keep this one open to keep working on a better fix.

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.

5 participants