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

Tests fail with latest Turing #134

Closed
ChrisRackauckas opened this issue Mar 21, 2020 · 13 comments
Closed

Tests fail with latest Turing #134

ChrisRackauckas opened this issue Mar 21, 2020 · 13 comments

Comments

@ChrisRackauckas
Copy link
Member

@Vaibhavdixit02 can you take a look at it?

@mohamed82008 is there any ways to make this more stable? It seems like Turing breaks their interface every month and this is really hard to keep up with.

@ChrisRackauckas ChrisRackauckas changed the title Tests fail with master Turing Tests fail with latest Turing Mar 21, 2020
@mohamed82008
Copy link
Contributor

I don't recall breaking anything in the last month. Let me see.

@mohamed82008
Copy link
Contributor

The problem seems to be with chains. There were a lot of changes in https://github.com/TuringLang/MCMCChains.jl recently.

@mohamed82008
Copy link
Contributor

CC: @cpfiffer @devmotion

@mohamed82008
Copy link
Contributor

I think we need a way to run downstream tests with new releases.

@mohamed82008
Copy link
Contributor

CC: @yebai @xukai92

@devmotion
Copy link
Member

I'm not sure which failing CI tests you're referring to exactly, do you have a link? The ones I saw still use MCMCChains 1.0.2 but the most recent version is 3.0.5. There were some errors related to extracting values from chains but they already showed up a month ago (see e.g. https://travis-ci.org/github/JuliaDiffEq/DiffEqBayes.jl/jobs/647741263) with MCMCChains 1.0.2, so these do not seem to be related to the most recent updates of MCMCChains. The recent versions of MCMCChains should fix some severe compilation time issues (see TuringLang/MCMCChains.jl#171), but unfortunately this required quite many changes. Sorry for that!

@ChrisRackauckas
Copy link
Member Author

I mean, that's kind of the issue though. It's breaking fast enough that there were two breaking changes since the last time we fixed it and it broke. We (are at least trying to) use this at a user level and not at a developer level, so I would assume that would be more stable. Are user codes with Turing breaking this much? Or are we using Turing incorrectly? If we're using Turing incorrectly, what's the stable API we should be using? If we could get some help getting something that's both updated and stable that would be great. I don't mean for this to come off as complaining, but I'm just curious how others are handling this.

@devmotion
Copy link
Member

I'll try to figure out what problems exactly are caused by the latest Turing version. I was just a bit confused since the CI test errors on master are caused by https://github.com/JuliaDiffEq/DiffEqBayes.jl/blob/0e84e8745c2d51a166e14d79cf29d6f1c85e66c1/src/stan_inference.jl#L71 which is completely unrelated to Turing.

In general, the latest version (0.9) is supposed to be a breaking release, as indicated by the version number, so it might very well require some updates in DiffEqBayes. However, as far as I can see (https://github.com/TuringLang/Turing.jl/releases), the main differences to 0.8.3 are additional features, bug fixes, and internal improvements. Also most changes on master should not affect the API but only be bug fixes, refactoring, and new features (such as automatic progress logging, use of AdvancedMH and EllipticalSliceSampling, and support of Zygote and ReverseDiff). But let's see what problems show up in #135.

@devmotion
Copy link
Member

Unfortunately the Turing tests are not executed due to the unrelated bug on master.

@devmotion
Copy link
Member

https://travis-ci.org/github/JuliaDiffEq/DiffEqBayes.jl/builds/665274434?utm_source=github_status&utm_medium=notification shows that the CI tests pass with Turing 0.9 (but you might want to use the keyword argument progress = false in the sample calls to avoid the verbose output of progress bars in Travis; you could even turn it off globally in Turing for the tests). The method definition warning in the beginning is fixed on Turing master, and the test error is still the unrelated issue mentioned above in the tests of Stan. So I guess this issue can be closed?

@ChrisRackauckas
Copy link
Member Author

Awesome. Interesting we got a failure on an older version, but we can just change the bound. Is there anything we need to update with MCMCChains?

@devmotion
Copy link
Member

Not that I know of, the API was not changed (or only unintentionally), there were just many internal changes. So I'd say, if something is broken, it's probably a bug and you should file an issue 😅 Since you do not even import MCMCChains here, it should be fine in DiffEqBayes.

@ChrisRackauckas
Copy link
Member Author

Looks like Stan also changed how it interacts with chains. Oh well, I'll tag since there's enough in here already.

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

No branches or pull requests

3 participants