Opened 7 months ago

Closed 7 months ago

#30954 closed enhancement (fixed)

Implement a proper equality check for polyhedron representation objects

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.3
Component: geometry Keywords: scalars, polyhedra
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: 9ee3335 (Commits, GitHub, GitLab) Commit: 9ee33351d8a20008f4ed6d24205962917c180561
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

sage: P = (1/2)*polytopes.cube()                                                                                                                                                    
sage: Q = (1/2)*polytopes.cube(backend='field')                                                                                                                                     
sage: for p in P.inequalities(): 
....:     assert p in Q.inequalities() 
....:                                                                                                                                                                               
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
<ipython-input-16-d0a8f3fbfdbb> in <module>
      1 for p in P.inequalities():
----> 2     assert p in Q.inequalities()
      3 

AssertionError: 

This isn't acceptable. Those inequalites are the same, only the defining vector differs by a positive scalar.

We also define a method is_facet_defining_inequality that checks whether self could replace an inequality of the polyhedron other. This is useful, when other is not full-dimensional and inequalities aren't unique (only after affine_hull_projection).

Change History (10)

comment:1 Changed 7 months ago by gh-kliem

  • Branch set to public/30954
  • Commit set to 4c1f2950e60a4d0a0ff4f46ebf5adb4247c20a7b
  • Summary changed from Implement a proper equality check for representation objects to Implement a proper equality check for polyhedron representation objects

New commits:

4c1f295scale rays, lines, inequalities and equations for comparison

comment:2 Changed 7 months ago by git

  • Commit changed from 4c1f2950e60a4d0a0ff4f46ebf5adb4247c20a7b to b1eac2c770c01961979580283c4500c42b56971b

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

b1eac2cimplement is_facet_defining_inequality

comment:3 Changed 7 months ago by gh-kliem

  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 follow-up: Changed 7 months ago by jipilab

  • Reviewers set to Jean-Philippe Labbé

Minor things:

-        cross_slack_matrix = self_matrix*hom_Vrep
+        cross_slack_matrix = self_matrix * hom_Vrep
-        if any(self.A()*line.vector() for line in other.lines()):
+        if any(self.A() * line.vector() for line in other.lines()):

In the TEST block, I would remove the QQ(*) to save some space... (could you also indent/line split?)

Why is there a lonely :: ?:

+            False
+
+        ::
+
+            sage: P = Polyhedron(vertices=[[0,0,0],[0,1,0]], lines=[[1,0,0]])

comment:5 in reply to: ↑ 4 Changed 7 months ago by gh-kliem

Replying to jipilab:

Why is there a lonely :: ?:

+            False
+
+        ::
+
+            sage: P = Polyhedron(vertices=[[0,0,0],[0,1,0]], lines=[[1,0,0]])

People use it to group examples, when they are too lazy to write down a nice sentence for each example.

comment:6 Changed 7 months ago by gh-kliem

It's just a new example, I redefine P, I redefine Q.

comment:7 Changed 7 months ago by jipilab

You forgot # optional - pynormaliz in the test-block.

comment:8 Changed 7 months ago by git

  • Commit changed from b1eac2c770c01961979580283c4500c42b56971b to 9ee33351d8a20008f4ed6d24205962917c180561

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

9ee3335reviewers comments

comment:9 Changed 7 months ago by jipilab

  • Status changed from needs_review to positive_review

Thanks! Looks good and will be useful!

comment:10 Changed 7 months ago by vbraun

  • Branch changed from public/30954 to 9ee33351d8a20008f4ed6d24205962917c180561
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.