Skip to content

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
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.

ChrisRackauckas added a commit that referenced this pull request Nov 9, 2024
Reverts part of #1067 because the manifest never got bumped. Keeping the update to the plots in single pendulums and the formatting change, but removing the package bumps for now since they were not used in the latest manifest and this will let CompatHelper track the real current state
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