-
Notifications
You must be signed in to change notification settings - Fork 223
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
Julia 1.7 tests are failing #2305
Comments
Oh, I'm guessing it's because ADTypes is a weak dependency of LogDensityProblemsAD, and Pkg 1.7 doesn't understand those as extensions were introduced in Julia 1.9. On Julia 1.10 if you try to do On the other hand, Julia 1.7 doesn't even think that ADTypes is a dependency of LogDensityProblemsAD so happily installs incompatible versions. |
Some possibly relevant context: At some point we dropped support for ADTypes v0.2, then ran into trouble with that, and reintroduced it. |
Ah that's very good to know. The issue isn't really with Turing, though. I think the "issue" is with LogDensityProblemsAD, because any package that uses julia<1.9 with ADTypes and LogDensityProblemsAD will run into the same issue. Even more generally, one would expect situations like these to happen with any package that uses extensions. Surely someone has solved this or run into this before? But I couldn't find anything on GitHub issues, Discourse, or Slack. |
I guess one problem is that LogDensityProblemsAD uses Requires to load the optional dependencies on Julia < 1.9, but does not e.g. load them unconditionally on older versions by making them proper dependencies on Julia < 1.9. Then dependencies and compat entries would be respected by Pkg also on Julia < 1.9 as expected. But it's somewhat understandable does LogDensityProblemsAD does not want to depend on all AD packages... I guess we can easily fix the current regression but the problem will come up with other updates again. Another problem on Julia < 1.9 is that SciML basically dropped support for Julia < 1.9 in almost all packages. Which makes it quite annoying and somewhat challenging to keep support Julia < 1.9 if you depend on parts of the SciML stack. So maybe we should generally consider dropping support for Julia < 1.9 (or not depend on SciML as much by default)? |
That sounds like a better long term solution. To clarify, why we depend on SciML? |
tpapp/LogDensityProblemsAD.jl#35 should be a quick hotfix for Julia < 1.9 + ADTypes 0.2. |
Thanks for the super quick fix @devmotion! Separately, I've posted on Discourse, https://discourse.julialang.org/t/compat-entries-for-weakdeps-on-julia-1-9/118172 but I'm not sure that there's a nice solution to it here. It appears to me the least bad way is for LogDensityProblemsAD to declare a requirement of julia >= 1.9, since that's the only way the compat entries will be respected. This is definitely suboptimal, because there are cases under which it could be compatible with < 1.9 and some people may well want to use that, but this is the only way to guarantee that it will work going forward. And I suppose it might be worth doing anyway based on how the Julia ecosystem is moving, though you all are the experts here. |
Oh, I see you just said the same thing there. :) Anyway, will rerun tests later today once the registry is updated and close this issue if they pass with the new version. |
All tests on Julia 1.7 are failing, see e.g. this run on #2304 https://github.com/TuringLang/Turing.jl/actions/runs/10378600942
I suspect it's because on Julia 1.7, ADTypes resolves to v0.2.7 (probably because some other dependency like SciMLBase specifies that), and that breaks when used with the new v1.9.1 of LogDensityProblemsAD (despite the compat entry being modified in tpapp/LogDensityProblemsAD.jl#34).
In contrast, on Julia 1.10, ADTypes resolves to v1.7.1 and all is good.
I'm not sure how Pkg handles version conflicts like these. I do find it weird that it resolved the environment to include both v1.9.1 of LogDensityProblemsAD and v0.2.7 of ADTypes, which are incompatible — intuitively, I'd have expected it to pick a lower version of LogDensityProblemsAD.
The text was updated successfully, but these errors were encountered: