-
-
Notifications
You must be signed in to change notification settings - Fork 112
fix: fix type-instability of get_root_indp
#1074
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
base: master
Are you sure you want to change the base?
Conversation
test/runtests.jl
Outdated
@@ -60,6 +60,9 @@ end | |||
@time @safetestset "Clocks" begin | |||
include("clock.jl") | |||
end | |||
@time @safetestset "JET" begin |
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.
Needs to go into a QA session.
Can this be merged? Similar to SciML/DiffEqBase.jl#1175 |
No it breaks a lot of tests in its current state and needs to get fixed first |
Makes sense. Just noting that this currently stops trimming (or at least JET being happy), but I'm happy to just use this PR branch for now. If others are looking for an alternative dirty fix that also seems to work, I've had success by pirating import NonlinearSolve.SciMLBase.get_root_indp
NonlinearSolve.SciMLBase.get_root_indp(prob::NonlinearLeastSquaresProblem) = prob.p |
It's safe to use. We just have a lot of legacy tests to fix up with this. In particular, we used to have an old method for passing an array of symbols This week we did an entire CI overhaul so we're a bit behind 😅 but we should be moving faster now that everything should be clean. |
bd2d461
to
ecc69be
Compare
ecc69be
to
c706c64
Compare
This would perhaps have been caught by SciML#665 but we're waiting on 1.12-rc2 and SciML/SciMLBase.jl#1074 to land before this would be automatically checked.
This would perhaps have been caught by #665 but we're waiting on 1.12-rc2 and SciML/SciMLBase.jl#1074 to land before this would be automatically checked.
@ChrisRackauckas @AayushSabharwal This PR is still a blocker for trimming and the trim test in SciML/NonlinearSolve.jl#665. Once this has been merged I will overhaul the trim test which I think will be quite nice to have landed. |
What is this currently stuck on? |
I haven't found a way to test this in a way that actually reflects how it is used and works across Julia versions. |
No description provided.