Opened 2 years ago
Closed 2 years ago
#29905 closed enhancement (fixed)
test basic properties of polyhedra
Reported by: | gh-kliem | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.2 |
Component: | geometry | Keywords: | polyhedra, test suite |
Cc: | jipilab, gh-LaisRast, dimpase, mjo | Merged in: | |
Authors: | Jonathan Kliem | Reviewers: | Matthias Koeppe |
Report Upstream: | N/A | Work issues: | |
Branch: | 699d4e8 (Commits, GitHub, GitLab) | Commit: | 699d4e8f6bd2dcffa0c751cb9b1fa01923f9232c |
Dependencies: | #29903 | Stopgaps: |
Description
We add a method that tests basic properties, when the TestSuite
is run.
Change History (16)
comment:1 Changed 2 years ago by
- Branch set to public/29905
- Commit set to 6eaf649147590f3a72ea04841470cb117533cca4
- Status changed from new to needs_review
comment:2 Changed 2 years ago by
- Reviewers set to Matthias Koeppe
- Status changed from needs_review to positive_review
comment:3 Changed 2 years ago by
Thank you.
comment:4 Changed 2 years ago by
- Cc dimpase mjo added
- Status changed from positive_review to needs_work
In light of the discussion on sage-devel (https://groups.google.com/d/msg/sage-devel/c4UbKSdt3Aw/UQAo1iYoAAAJ), this needs more work
comment:5 Changed 2 years ago by
Why? What's wrong with this ticket?
Note that this ticket really has only one commit. If there is something wrong with #29903 or #29904 it should be fixed there. Not that I would know what the problem with those tickets would be.
One thing I could do is to use this ticket to improve the doctest for the hypercubes. Is that what you mean? Somethings as
sage: P = polytopes.hypercube(intervals=random_intervals, backend='field') sage: P._test_basic_properties()
comment:6 Changed 2 years ago by
The set_random_seed
added in the doctest.
comment:7 Changed 2 years ago by
comment:8 Changed 2 years ago by
Please feel free to move the "needs work" to the correct ticket. Thanks
comment:9 Changed 2 years ago by
Yes, if you are concerned about the random seeds, this needs a proper solution, not one that is localized to one doctest.
comment:10 follow-up: ↓ 12 Changed 2 years ago by
By the way, if you want to fuzz this particular doctest, why not just execute it in a loop so it picks up a new set of deterministic pseudo random numbers in each iteration? That should be good enough.
comment:11 Changed 2 years ago by
- Commit changed from 6eaf649147590f3a72ea04841470cb117533cca4 to 699d4e8f6bd2dcffa0c751cb9b1fa01923f9232c
Branch pushed to git repo; I updated commit sha1. New commits:
699d4e8 | remove set_random seed
|
comment:12 in reply to: ↑ 10 Changed 2 years ago by
Replying to mkoeppe:
By the way, if you want to fuzz this particular doctest, why not just execute it in a loop so it picks up a new set of deterministic pseudo random numbers in each iteration? That should be good enough.
I already added a new doctest for #29904 that manually tests the only interesting case besides the static case that is always being tested.
comment:13 Changed 2 years ago by
- Status changed from needs_work to needs_review
comment:14 Changed 2 years ago by
- Status changed from needs_review to positive_review
comment:15 Changed 2 years ago by
Thank you.
comment:16 Changed 2 years ago by
- Branch changed from public/29905 to 699d4e8f6bd2dcffa0c751cb9b1fa01923f9232c
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
test suites for the library
fix double description of hypercube
merged in public/29904
added long time flags
some more testsuites
test some basic properties