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

Fix ODE compatibility issues for now #607

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Conversation

johnzl-777
Copy link
Collaborator

in 1.9 the latest version of OrdinaryDiffEq seems to have compatibility issues with our custom __init function. I've set the compatibility to the last compatible version which seems to be anything before 6.58.0.

@johnzl-777 johnzl-777 requested a review from weinbe58 October 20, 2023 21:00
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #607 (2e626c9) into master (e614345) will increase coverage by 3.83%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #607      +/-   ##
==========================================
+ Coverage   71.21%   75.05%   +3.83%     
==========================================
  Files          53       80      +27     
  Lines        3148     4346    +1198     
==========================================
+ Hits         2242     3262    +1020     
- Misses        906     1084     +178     

see 27 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@weinbe58 weinbe58 left a comment

Choose a reason for hiding this comment

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

Seems like there is still an issue related to __init in BloqadeKrylov tests. Maybe we need to lower the version more?

@ArbitRandomUser
Copy link

did BlockadeKrylov fail because its test environment still uses BloqadeODE from the registry ?

@ArbitRandomUser
Copy link

ArbitRandomUser commented Nov 5, 2023

BloqadeExpr exports SumOfLoop now which the registry package does not have yet. This also causes tests to fail.

setting up a custom test environment (deving packages that Project.toml includes from lib) and manually include("runtests.jl") in it causes one more error with ForwardDiff.jl . which is fixed by updating ForwardDiff to v0.10.36.
nvm mind that , idk how it somehow got stuck on v0.10.32 before ,

the environment i used for running the test ...

Status `~/workspace/Bloqade.jl/lib/BloqadeKrylov/test/Project.toml`
  [bd27d05e] BloqadeExpr v0.1.14 `../../BloqadeExpr`
  [bd27d05e] BloqadeKrylov v0.1.8 `..`
  [bd27d05e] BloqadeLattices v0.1.8 `../../BloqadeLattices`
  [bd27d05e] BloqadeODE v0.1.11 `../../BloqadeODE`
  [bd27d05e] BloqadeWaveforms v0.1.7 `../../BloqadeWaveforms`
  [d4d017d3] ExponentialUtilities v1.25.0
  [f6369f11] ForwardDiff v0.10.36
  [5872b779] Yao v0.8.11
  [e600142f] YaoArrayRegister v0.9.7
  [bd27d05e] YaoSubspaceArrayReg v0.1.8
  [37e2e46d] LinearAlgebra
  [9a3f8284] Random
  [2f01184e] SparseArrays
  [8dfed614] Test

@weinbe58
Copy link
Member

weinbe58 commented Nov 6, 2023

The problem is that one of the tests depends on BloqadeODE which is currently broken by the compatibility. Its a test dependency so that "dev"ing the local package doesn't cover that dependency so I think we need to just merge this first and release a new version of BloqadeODE before we can fix the CI.

@weinbe58 weinbe58 merged commit 0759d57 into master Nov 6, 2023
21 of 27 checks passed
@weinbe58 weinbe58 deleted the john/fix-ode-compat branch November 6, 2023 14:52
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