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.

Sign up for GitHub

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

Jump to bottom

Apply W-transform to Rosenbrock23 #2307

Merged
merged 7 commits into from
Sep 12, 2024
Merged

Apply W-transform to Rosenbrock23 #2307

merged 7 commits into from
Sep 12, 2024

Conversation

ChrisRackauckas
Copy link
Member

This reverts commit d125014.

@ChrisRackauckas
Copy link
Member Author

@oscardssmith here's a rebased version of your commit.

@ChrisRackauckas
Copy link
Member Author

You didn't handle ode_addsteps!

@oscardssmith
Copy link
Contributor

I don't believe ode_addsteps! needed changes. The way I made the change didn't change any of the ks so everything should work the same outside the perform_step!.

@oscardssmith
Copy link
Contributor

yeah also CI seems green for all the OrdinaryDiffEq tests.

@ChrisRackauckas
Copy link
Member Author

DelayDiffEq downstream breaks, RegressionI and IntegratorsII breaks.

@ChrisRackauckas
Copy link
Member Author

Add steps recomputes the k's so I am not sure how that makes sense

@ChrisRackauckas
Copy link
Member Author

https://buildkite.com/julialang/ordinarydiffeq-dot-jl/builds/4809#01912de7-dcf3-4a60-899d-be1327fd30a4/906-1470

@oscardssmith
Copy link
Contributor

ok I think this is ready to go. Let's see what CI says.

@oscardssmith
Copy link
Contributor

@ChrisRackauckas can you decipher the DDE failures here? They look related, but I don't understand the cause.

@ChrisRackauckas
Copy link
Member Author

I must say that error message is a bit puzzling.

@oscardssmith
Copy link
Contributor

Turns out the error was quite simple. I had just forgotten to switch the build_jac_config to be built for W transforms (and we have no testing of Wfact or Wfact_t in all of the OrdinaryDiffEq test suite.

@oscardssmith oscardssmith mentioned this pull request Sep 12, 2024
Merged
@oscardssmith
Copy link
Contributor

Ok, this appears to be ready to merge.

@oscardssmith oscardssmith merged commit e5dd0ff into master Sep 12, 2024
57 of 61 checks passed
@oscardssmith oscardssmith deleted the wtransform branch September 12, 2024 18:01
@oscardssmith oscardssmith mentioned this pull request Sep 14, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ChrisRackauckas @oscardssmith