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

New minor versions of DPPL can't be tested #740

Closed
penelopeysm opened this issue Dec 7, 2024 · 4 comments
Closed

New minor versions of DPPL can't be tested #740

penelopeysm opened this issue Dec 7, 2024 · 4 comments

Comments

@penelopeysm
Copy link
Member

penelopeysm commented Dec 7, 2024

#727 had a slightly hidden consequence: it declares Mooncake as a DPPL test dep, but Mooncake also has a compat bound on DPPL=0.31 (https://github.com/compintell/Mooncake.jl/blob/main/Project.toml) – so if the minor version of DPPL is bumped, it's impossible to have a consistent test environment, e.g. https://github.com/TuringLang/DynamicPPL.jl/actions/runs/12208921847/job/34062921291?pr=733

@willtebbutt, I'm not immediately sure what's the best way around this. Would it be sensible to move MooncakeDynamicPPLExt to DynamicPPLMooncakeExt instead?

I guess the other way round would be to move any tests involving Mooncake into a separate script (and/or CI job) and only run them if the compat entries can be resolved (urk).

@willtebbutt
Copy link
Member

Ah, yes, this isn't ideal.

The MooncakeDynamicPPLExt is just a one-line performance optimisation at this point, see here, so maybe it should just go in this package?

@penelopeysm
Copy link
Member Author

I think that looks like the best fix to me! I was a bit worried you might end up with the reverse problem if you bump Mooncake's minor version, but looking at it a bit I think you're safe since the main DPPL package itself doesn't have a compat entry for Mooncake?

Would you like to do the honours, or shall I?

@willtebbutt
Copy link
Member

Yeah. Additionally in Mooncake we'll now just have integration tests involving DPPL, and they're completely isolated from the main test environment (and don't actually have compat entries for the deps), so we should have nicely decoupled everything.

I think the correct sequencing is probably

  1. Remove the DPPL test dep + release a new version of Mooncake,
  2. Make a PR to DPPL to add the one-liner + bump the Mooncake compat.

I'll make the Mooncake PR now!

This was referenced Dec 7, 2024
@penelopeysm
Copy link
Member Author

Closed by the above two PRs ^

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

2 participants