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

test basic properties of polyhedra #29905

Closed
kliem opened this issue Jun 19, 2020 · 17 comments
Closed

test basic properties of polyhedra #29905

kliem opened this issue Jun 19, 2020 · 17 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jun 19, 2020

We add a method that tests basic properties, when the TestSuite is run.

Depends on #29903

CC: @jplab @LaisRast @dimpase @orlitzky

Component: geometry

Keywords: polyhedra, test suite

Author: Jonathan Kliem

Branch/Commit: 699d4e8

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/29905

@kliem kliem added this to the sage-9.2 milestone Jun 19, 2020
@kliem
Copy link
Contributor Author

kliem commented Jun 19, 2020

Commit: 6eaf649

@kliem
Copy link
Contributor Author

kliem commented Jun 19, 2020

Branch: public/29905

@kliem
Copy link
Contributor Author

kliem commented Jun 19, 2020

New commits:

0bdbe49test suites for the library
5c7e562fix double description of hypercube
206dbb7merged in public/29904
c6ea1ecadded long time flags
5d79b2bsome more testsuites
6eaf649test some basic properties

@mkoeppe
Copy link
Member

mkoeppe commented Jun 20, 2020

Reviewer: Matthias Koeppe

@kliem
Copy link
Contributor Author

kliem commented Jun 20, 2020

comment:3

Thank you.

@mkoeppe
Copy link
Member

mkoeppe commented Jun 22, 2020

comment:4

In light of the discussion on sage-devel (https://groups.google.com/d/msg/sage-devel/c4UbKSdt3Aw/UQAo1iYoAAAJ), this needs more work

@kliem
Copy link
Contributor Author

kliem commented Jun 22, 2020

comment:5

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()

@mkoeppe
Copy link
Member

mkoeppe commented Jun 22, 2020

comment:6

The set_random_seed added in the doctest.

@kliem
Copy link
Contributor Author

kliem commented Jun 22, 2020

comment:7

If that is the problem, then this ticket here is still green and #29903 should be "needs work". To avoid confusion I can of course quickly push one commit to all three tickets.

So we don't put set_random_seed anymore because we will go through with #29935 soon?

@mkoeppe
Copy link
Member

mkoeppe commented Jun 22, 2020

comment:8

Please feel free to move the "needs work" to the correct ticket. Thanks

@mkoeppe
Copy link
Member

mkoeppe commented Jun 22, 2020

comment:9

Yes, if you are concerned about the random seeds, this needs a proper solution, not one that is localized to one doctest.

@mkoeppe
Copy link
Member

mkoeppe commented Jun 22, 2020

comment:10

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

699d4e8remove set_random seed

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 22, 2020

Changed commit from 6eaf649 to 699d4e8

@kliem
Copy link
Contributor Author

kliem commented Jun 22, 2020

comment:12

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.

@kliem
Copy link
Contributor Author

kliem commented Jun 26, 2020

comment:15

Thank you.

@vbraun
Copy link
Member

vbraun commented Jul 10, 2020

Changed branch from public/29905 to 699d4e8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants