Opened 8 years ago
Last modified 8 years ago
#14205 needs_info enhancement
polybori doctests involve randomness
Reported by: | tkluck | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | minor | Milestone: | sage-wishlist |
Component: | algebra | Keywords: | |
Cc: | AlexanderDreyer | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | None of the above - read trac for reasoning. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
As discusses in #13767, there is some doctests for the random_set
function in polybori that have inherent random results. Because of that, they might break if the implementation of random functions changes, such as what happened in that ticket.
This is a rare occasion, but still it might be worthwhile to change these kind of doctests to only check for documented behavior.
For example, the doctests could check that result = random_set(monomial, n)
should obey len(result) == n
and result.diff(monomial.divisors()).empty()
. (suggested by Alexander Dreyer)
Or we could test against theorems (such as: One has random data, but the test is using an identity that must hold for any data). (suggested by Simon King)
Change History (7)
comment:1 follow-up: ↓ 2 Changed 8 years ago by
comment:2 in reply to: ↑ 1 Changed 8 years ago by
Replying to vbraun:
I'd rather have the output of boost.Random doctested so we know when the implementation changes. We also test all other RNGs to verify that the sequence that they produce is actually the same, that is, we set the seed correctly.
Well, I understood from the discussion here, that the test should not depend on the random generator: https://groups.google.com/forum/?fromgroups=#!topic/sage-devel/uSEWZrE-T_8
Also, sage-on-gentoo and sage-on-debian may use different versions of boost than the original Sage.
An alternative could be that we additionally add (and document) the generic tests and keep the explicit ones. This would simplify the bug hunt next time.
comment:3 Changed 8 years ago by
- Report Upstream changed from N/A to None of the above - read trac for reasoning.
comment:4 Changed 8 years ago by
- Status changed from new to needs_review
Doctesting the random sequence (see sage.misc.randstate
) and doctesting the correctness of the output are two different things. And ideally we'd have both, though its better to have randstate-dependent tests than none at all.
comment:5 Changed 8 years ago by
- Milestone changed from sage-5.8 to sage-wishlist
comment:6 Changed 8 years ago by
- Status changed from needs_review to needs_info
So you would just keep it, like it was in #13767?
comment:7 Changed 8 years ago by
Correct me if I'm wrong, but the failing doctests were pretty much directly tests of the random number generator. If thats the case then I'd keep them since they test the rng seeding.
I'd rather have the output of boost.Random doctested so we know when the implementation changes. We also test all other RNGs to verify that the sequence that they produce is actually the same, that is, we set the seed correctly.