Opened 2 years ago
Closed 2 years ago
#29905 closed enhancement (fixed)
test basic properties of polyhedra
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  geometry  Keywords:  polyhedra, test suite 
Cc:  jipilab, ghLaisRast, 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 sagedevel (https://groups.google.com/d/msg/sagedevel/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 followup: ↓ 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