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

DynamicalODE Changes #1067

Merged
merged 10 commits into from
Oct 19, 2024
Merged

Conversation

ParamThakkar123
Copy link
Contributor

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

Add any other context about the problem here.

@ChrisRackauckas
Copy link
Member

No that's backwards in compat.

@ParamThakkar123
Copy link
Contributor Author

@ChrisRackauckas

@ParamThakkar123
Copy link
Contributor Author

@ChrisRackauckas any changes needed further ??

@ChrisRackauckas
Copy link
Member

Error: UndefVarError: singlependulum not defined.

@ChrisRackauckas
Copy link
Member

Error: UndefVarError: plotsolenergy not defined

@ParamThakkar123
Copy link
Contributor Author

Error: UndefVarError: singlependulum not defined.

I think this is already present here :

https://github.com/SciML/SciMLBenchmarks.jl/blob/master/benchmarks/DynamicalODE/single_pendulums.jmd

Where can I make an update if this needs a fix ??

@ChrisRackauckas
Copy link
Member

The point of the project is to get that benchmark set working on the latest versions 😅 . That's in the benchmark script of the folder.

@ParamThakkar123
Copy link
Contributor Author

Oh, Got it 😅

@ParamThakkar123
Copy link
Contributor Author

But my question was why did we get a undef error if that's already defined there ??

@ChrisRackauckas
Copy link
Member

Previous bad bump maybe?

@ParamThakkar123
Copy link
Contributor Author

@ChrisRackauckas How can I see the errors that have occurred ??

Error: UndefVarError: plotsolenergy not defined

Like here

@ErikQQY
Copy link
Member

ErikQQY commented Oct 18, 2024

@ParamThakkar123 Check the README of this repo: https://github.com/SciML/SciMLBenchmarks.jl?tab=readme-ov-file#inspecting-benchmark-results, and the built benchmarks can be checked in the corresponding md files

@ErikQQY
Copy link
Member

ErikQQY commented Oct 18, 2024

Seems the errors are caused by PyPlot.jl

You may consider figuring out what's wrong with PyPlot.jl here or change to Plots.jl(recommend)

@ParamThakkar123
Copy link
Contributor Author

Seems the errors are caused by PyPlot.jl

You may consider figuring out what's wrong with PyPlot.jl here or change to Plots.jl(recommend)

The errors says that the figure function is not defined. Though it's there in PyPlot

@ErikQQY
Copy link
Member

ErikQQY commented Oct 18, 2024

You can try to update the compat and the Manifest of the packages in this benchmark to the latest version, many of them are outdated

@ChrisRackauckas
Copy link
Member

It would probably be best to just update the plots to use Plots.jl

@ParamThakkar123
Copy link
Contributor Author

It would probably be best to just update the plots to use Plots.jl

Noted. I will make the changes

@ChrisRackauckas ChrisRackauckas merged commit 1480575 into SciML:master Oct 19, 2024
2 checks passed
@@ -2345,4 +2345,4 @@ version = "3.5.0+0"
deps = ["Artifacts", "JLLWrappers", "Libdl", "Pkg", "Wayland_jll", "Wayland_protocols_jll", "Xorg_libxcb_jll", "Xorg_xkeyboard_config_jll"]
git-tree-sha1 = "9c304562909ab2bab0262639bd4f444d7bc2be37"
uuid = "d8fb68d0-12a3-5cfd-a85a-d49703b185fd"
version = "1.4.1+1"
version = "1.4.1+1"
Copy link
Member

Choose a reason for hiding this comment

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

this manifest bump is not correct!

@ChrisRackauckas
Copy link
Member

The updates to the plots are correct, but the manifest was not bumped so this isn't actually testing the newer versions. Can you bump the manifest? Otherwise we need to revert.

@ParamThakkar123
Copy link
Contributor Author

Yes @ChrisRackauckas , I will create a new pr by today which bumps the manifest with the latest versions for both DynamicalODE and BayesianInference.

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.

3 participants