Skip to content

Conversation

@jClugstor
Copy link
Member

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

This changes the tests to use the new things like t_nounits and D_nounits instead of declaring a parameter t and Differential for every model. This makes it more idiomatic.

Also changed the datafit tests to use the normal Lorenz system instead of the modified one. I think some issues with the CI were being caused because the bayesian_datafits were testing a lot of parameters that made the system difficult to solve, which caused the CI to time out. This also meant I had to give the optimizers a few more timesteps to be in tolerance, but the tests run much faster now.

This also updates to Turing 0.33. I could try to seperate that out, but for some reason when I try to downgrade to 0.30 to test I get precompilation errors that I haven't been able to work out.

@ChrisRackauckas
Copy link
Member

Also changed the datafit tests to use the normal Lorenz system instead of the modified one.

That's a chaotic system. Did you use short time?

test/datafit.jl Outdated
sol = solve(prob)

tsave = [1.0, 2.0, 3.0]
tsave = [1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0]
Copy link
Member

Choose a reason for hiding this comment

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

beyond a lyopunov time will not give good results for a chaotic system.

If you want to change the model, I'd suggest lotka-volterra as a better example

Copy link
Member Author

Choose a reason for hiding this comment

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

Lorenz should be fine after 10 seconds which is why it passes, but no reason to have them be chaotic in the first place, so I'll change it to LK

@ChrisRackauckas
Copy link
Member

Can you follow up with a doc fix?

@ChrisRackauckas ChrisRackauckas merged commit 529a178 into SciML:main Jul 27, 2024
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.

2 participants