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

Docs Failing Test #562

Closed
dylan-asmar opened this issue Oct 16, 2024 · 4 comments · Fixed by #563
Closed

Docs Failing Test #562

dylan-asmar opened this issue Oct 16, 2024 · 4 comments · Fixed by #563

Comments

@dylan-asmar
Copy link
Member

The docs are failing (caught with our weekly automatic run).

I have included the error below. There were some changes with Random in the 1.11 release. However, I didn't dig too deeply into the changes.

If this part of the docs is testing functionality, then I think we can remove the test for the exact value?

┌ Error: doctest failure in src/def_pomdp.md:218-228
│ 
│ ```jldoctest
│ using Random: MersenneTwister
│ using POMDPTools: ImplicitDistribution
│ 
│ rng = MersenneTwister(1)
│ 
│ d = ImplicitDistribution(rng -> (-0.2*rand(rng), 0.0))
│ rand(rng, d)
│ # output
│ (-0.047206669[13](https://github.com/JuliaPOMDP/POMDPs.jl/actions/runs/11318604248/job/31596414110#step:5:14)240939, 0.0)
│ ```
│ 
│ Subexpression:
│ 
│ using Random: MersenneTwister
│ using POMDPTools: ImplicitDistribution
│ 
│ rng = MersenneTwister(1)
│ 
│ d = ImplicitDistribution(rng -> (-0.2*rand(rng), 0.0))
│ rand(rng, d)
│ 
│ Evaluated output:
│ 
│ (-0.019827940275727363, 0.0)
│ 
│ Expected output:
│ 
│ (-0.04720666913240939, 0.0)
│ 
│   diff =
│    Warning: Diff output requires color.
│    (-0.04720666913240939, (-0.019827940275727363, 0.0)
└ @ Documenter src/def_pomdp.md:2[18](https://github.com/JuliaPOMDP/POMDPs.jl/actions/runs/11318604248/job/31596414110#step:5:19)
@mykelk
Copy link
Member

mykelk commented Oct 17, 2024

Unless we want to switch to StableRNGs, I think we should probably relax the exactness.

@dylan-asmar
Copy link
Member Author

I'm ok with any change. I didn't write this test so I defer to others that might have insight.

@mykelk
Copy link
Member

mykelk commented Oct 17, 2024

Cool. Which way should we go @zsunberg ?

@zsunberg
Copy link
Member

We should use the stable rng. That is what it's meant for.

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 a pull request may close this issue.

3 participants