#30605 closed enhancement (fixed)

improve cone containment tests

Reported by: mjo Owned by:
Priority: major Milestone: sage-9.2
Component: geometry Keywords:
Cc: gh-kliem, jipilab, novoselt Merged in:
Authors: Michael Orlitzky Reviewers: Jonathan Kliem
Report Upstream: N/A Work issues:
Branch: 7ea2dd8 (Commits, GitHub, GitLab) Commit: 7ea2dd880b8a3047a05be8bb1d521b48e134cf73
Dependencies: Stopgaps:

Status badges

Description

If K is a polyhedral cone, the test x in K only supports x with rational coordinates. It is desirable in many cases to perform the same test with irrational numbers as well.

This will simplify the implementation in ticket #29169.

Change History (9)

comment:1 Changed 11 months ago by novoselt

I was thinking what should be done with reals and other non-exact things, but I think it does not matter - as long as exact ones are handled correctly, it is user's responsibility to be aware that non-exact things may give wrong results without extra care.

comment:2 Changed 11 months ago by mjo

The difficulty is never where you think it will be...

sage: bool(I >= 0)                                                              
True

comment:3 Changed 11 months ago by mjo

  • Branch set to u/mjo/ticket/30605
  • Commit set to 507affb163b2113e1b77d6858efaf65e2170a46d
  • Status changed from new to needs_review

Maybe this suffices?


New commits:

cf36516Trac #30605: try AA before RR in cone._ambient_space_point().
276db84Trac #30605: catch more coercion failures in cone._ambient_space_point().
507affbTrac #30605: factor out repetition in cone._ambient_space_point().

comment:4 Changed 11 months ago by mjo

  • Authors set to Michael Orlitzky

comment:5 Changed 11 months ago by git

  • Commit changed from 507affb163b2113e1b77d6858efaf65e2170a46d to 09c2fa3903f4e6f1e5d06bc1fbcbad519521cf8e

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

09c2fa3Trac #30605: update polyhedral cone containment documentation.

comment:6 Changed 11 months ago by gh-kliem

  • Reviewers set to Jonathan Kliem

Could you follow the recommendation of pycodestyle and do one command per line, e.g.

-        if p is not None: return p
+        if p is not None:
+            return p

or

-        try: return L.base_extend(ring)(data)
-        except TypeError: pass
-        except ValueError as ex:
-            if str(ex).startswith("Cannot coerce"): pass
+        try:
+            return L.base_extend(ring)(data)
+        except TypeError:
+            pass
+        except ValueError as ex:
+            if str(ex).startswith("Cannot coerce"):
+                pass

I think this make it easier to read.

Once done, you can set in on positive review on my behalf.

comment:7 Changed 11 months ago by git

  • Commit changed from 09c2fa3903f4e6f1e5d06bc1fbcbad519521cf8e to 7ea2dd880b8a3047a05be8bb1d521b48e134cf73

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

7ea2dd8Trac #30605: reformat according to pycodestyle.

comment:8 Changed 11 months ago by mjo

  • Status changed from needs_review to positive_review

Thanks! I fixed the indentation in one final (separate) commit if you want to take a look. But pycodestyle is happy now.

comment:9 Changed 10 months ago by vbraun

  • Branch changed from u/mjo/ticket/30605 to 7ea2dd880b8a3047a05be8bb1d521b48e134cf73
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.