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

Robustify doctest in hyperbolic_geodesic.py #23582

Closed
rwst opened this issue Aug 5, 2017 · 8 comments
Closed

Robustify doctest in hyperbolic_geodesic.py #23582

rwst opened this issue Aug 5, 2017 · 8 comments

Comments

@rwst
Copy link

rwst commented Aug 5, 2017

A doctest of the midpoint code in geometry/hyperbolic_space/hyperbolic_geodesic.py relies on exact reproduction of complicated symbolic expressions involving square roots. With recent changes in Pynac the representation of the result no longer matches, although it is mathematically identical.

The ticket changes the doctest to match results numerically.

Component: geometry

Author: Ralf Stephan

Branch/Commit: u/rws/robustify_doctest_in_hyperbolic_geodesic_py @ a90ce6a

Reviewer: Frédéric Chapoton

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

@rwst rwst added this to the sage-8.1 milestone Aug 5, 2017
@rwst
Copy link
Author

rwst commented Aug 5, 2017

@rwst
Copy link
Author

rwst commented Aug 5, 2017

Author: Ralf Stephan

@rwst
Copy link
Author

rwst commented Aug 5, 2017

Commit: a90ce6a

@rwst
Copy link
Author

rwst commented Aug 5, 2017

New commits:

a90ce6a23582: Robustify doctest in hyperbolic_geodesic.py

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:3

ok, looks good

@tscrim
Copy link
Collaborator

tscrim commented Aug 9, 2017

comment:4

I'm not completely sure about this. If it is mathematically identical, why not simply change the doctest with the new version of pynac? Should then all (sufficiently complicated) symbolic expressions output not be used for doctests? IMO, this makes the doctest less likely to catch errors and feels like a tiny step backwards in that regard.

@rwst
Copy link
Author

rwst commented Aug 10, 2017

comment:5

The change is no longer necessary because the mentioned change in Pynac didn't happen. I agree to make the ticket invalid.

However, let me point out that it is highly unlikely that a symbolic error would pass such a numeric test. Note that such numeric tests additionally test the FP evaluation code so it's more likely to catch bugs.

@rwst rwst removed this from the sage-8.1 milestone Aug 10, 2017
@embray embray closed this as completed Dec 12, 2017
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

4 participants