#29569 closed enhancement (fixed)

Obtain polar with both Vrep and Hrep (if backend supports it)

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.2
Component: geometry Keywords: polar, polytopes
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: c4f9dba (Commits, GitHub, GitLab) Commit: c4f9dbac6a8eeb3a77384a46fd06d9ce0ffdc3ee
Dependencies: #29568 Stopgaps:

Status badges

Description (last modified by gh-kliem)

We obtain the polar with both Vrep and Hrep to speed things up.

Along the way we optimize translation a bit in the spirit of #28866 and we outsource obtaining the new double description from this method.

Now one can obtain the new data with _translation_double_description without actually creating anything.

Change History (21)

comment:1 Changed 17 months ago by gh-kliem

  • Description modified (diff)

comment:2 Changed 17 months ago by gh-kliem

  • Branch set to public/29569
  • Commit set to 69513eb0e33d8b4ba3426b067f76dae784edf0f5

New commits:

649153aget only the new double descpription from translation
89d4dd3documentation for `_translation_double_description`
69513ebuse precomputed data for polar

comment:3 Changed 17 months ago by gh-kliem

  • Description modified (diff)

comment:4 Changed 17 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

comment:5 Changed 17 months ago by gh-kliem

  • Branch changed from public/29569 to public/29569-reb
  • Commit changed from 69513eb0e33d8b4ba3426b067f76dae784edf0f5 to fc0a37571cc261c126ec6550e891e04189a80811
  • Status changed from new to needs_review

New commits:

aa5ef80initalize cdd with both Hrep and Vrep to deal with numerical inconsistency
4a3022eget only the new double descpription from translation
422d655documentation for `_translation_double_description`
cabd811use precomputed data for polar
fc0a375fix doctets

comment:6 Changed 17 months ago by gh-kliem

  • Branch changed from public/29569-reb to public/29569-reb2
  • Commit changed from fc0a37571cc261c126ec6550e891e04189a80811 to 758902c944cc17207f953298afad238856bb1804

New commits:

7e5ecafinitalize cdd with both Hrep and Vrep to deal with numerical inconsistency
6cedfb6get only the new double descpription from translation
803b6acdocumentation for `_translation_double_description`
3e4ab32use precomputed data for polar
8bbd5cbfix doctets
a39af8bfixed failing doctest
758902cremoved redundant import

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

About the bug that you mentioned, it is really a bug?

I am asking because "polar" means different things depending on which corner of the street you stand at...

sage: p = LatticePolytope([[1,0],[0,1],[-1,-1]])
sage: p
2-d reflexive polytope #0 in 2-d lattice M
sage: p.polar()
2-d reflexive polytope #15 in 2-d lattice N
sage: p.polar().vertices()
N( 2, -1),
N(-1,  2),
N(-1, -1)
in 2-d lattice N

comment:8 Changed 17 months ago by jipilab

  • Status changed from needs_review to needs_work

In the tutorial, it would be good to still have an example where it really still gives an error. There is likely such an example in the library (see those that are forced to be exact).

I would then keep a error and a warning in the tutorial to maintain an up-to-date document.

comment:9 in reply to: ↑ 7 Changed 17 months ago by gh-kliem

Replying to jipilab:

About the bug that you mentioned, it is really a bug?

I am asking because "polar" means different things depending on which corner of the street you stand at...

sage: p = LatticePolytope([[1,0],[0,1],[-1,-1]])
sage: p
2-d reflexive polytope #0 in 2-d lattice M
sage: p.polar()
2-d reflexive polytope #15 in 2-d lattice N
sage: p.polar().vertices()
N( 2, -1),
N(-1,  2),
N(-1, -1)
in 2-d lattice N

Thanks for checking this. No it's not a bug. It's exactly the output described in OUTPUT.

It's still confusing. But this ticket shouldn't be about making things more uniform.

comment:10 Changed 17 months ago by gh-kliem

  • Description modified (diff)

comment:11 Changed 17 months ago by git

  • Commit changed from 758902c944cc17207f953298afad238856bb1804 to 81cf70f32da929c2d6b8dba5756bd936e4ad0072

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

630e83aundo non-fix for polar ZZ
81cf70ffix limitations of RDF example

comment:12 Changed 17 months ago by gh-kliem

  • Status changed from needs_work to needs_review

comment:13 Changed 17 months ago by jipilab

The fix in the last commit means that it was no passing the tests? It would also be nice to have the "warning" case and the "error" case if we can find the two in the library for example.

comment:14 Changed 17 months ago by gh-kliem

If you sort it, you get an error. If you don't sort it, you get a warning.

comment:15 Changed 17 months ago by git

  • Commit changed from 81cf70f32da929c2d6b8dba5756bd936e4ad0072 to c4f9dbac6a8eeb3a77384a46fd06d9ce0ffdc3ee

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

c4f9dbagive both examples: Error and Warning

comment:16 Changed 17 months ago by gh-kliem

Now I'm giving both examples.

comment:17 Changed 17 months ago by jipilab

  • Reviewers set to Jean-Philippe Labbé
  • Status changed from needs_review to positive_review

Looks good to me now. The code styles are fixed in a different ticket.

comment:18 Changed 17 months ago by gh-kliem

Thank you.

comment:19 follow-up: Changed 17 months ago by gh-kliem

@jipilab: Are you ware that the first commit of this ticket is actually #29568.

comment:20 in reply to: ↑ 19 Changed 17 months ago by jipilab

Replying to gh-kliem:

@jipilab: Are you ware that the first commit of this ticket is actually #29568.

I missed that. I'll have a look.

comment:21 Changed 16 months ago by vbraun

  • Branch changed from public/29569-reb2 to c4f9dbac6a8eeb3a77384a46fd06d9ce0ffdc3ee
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.