Sage: Ticket #30248: Normaliz backend is broken with double description input
https://trac.sagemath.org/ticket/30248
<p>
This bug was found while manipulating hyperplane arrangements.
</p>
<p>
A minimal example to reproduce the bug is:
</p>
<pre class="wiki">sage: p1 = Polyhedron(backend='normaliz', base_ring=AA, rays=[(AA(0), AA(0), AA(1)), (AA(0), AA(1), AA(-1)), (AA(1), AA(0), AA(-1))], vertices=[(AA(0), AA(0), AA(0))])
sage: p1
A 3-dimensional polyhedron in AA^3 defined as the convex hull of 1 vertex and 3 rays
sage: -p1
A 3-dimensional polyhedron in AA^3 defined as the convex hull of 1 vertex and 3 lines
sage: p2 = Polyhedron(backend='normaliz', base_ring=AA, rays=[(AA(-1), AA(0), AA(1)), (AA(-1), AA(1), AA(0)), (AA(0), AA(0), AA(-1))], vertices=[(AA(0), AA(0), AA(0))])
sage: p2
A 3-dimensional polyhedron in AA^3 defined as the convex hull of 1 vertex and 3 rays
sage: -p2
A 3-dimensional polyhedron in AA^3 defined as the convex hull of 1 vertex and 3 lines
</pre><p>
Looking at <code>dilation</code> it seems that dilation is not the problem, it does the right thing, but the <code>parent.element_class</code> call messes things up.
</p>
<p>
Notice that changing the base ring to <code>QQ</code> or <code>ZZ</code> or removing the <code>'normaliz'</code> backend, one does not get the error... This is nasty.
</p>
<p>
In the hyperplane arrangement, there are some rational regions, and some irrational regions...
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/30248
Trac 1.1.6gh-kliemWed, 29 Jul 2020 15:52:35 GMT
https://trac.sagemath.org/ticket/30248#comment:1
https://trac.sagemath.org/ticket/30248#comment:1
<p>
Notice that normaliz doesn't use precomputed double description yet (waiting for the package upgrade). I suspect intitialzing from inequaltities doesn't work correctly and there is an even simpler one line example for this.
</p>
Ticketgh-kliemWed, 29 Jul 2020 16:11:27 GMT
https://trac.sagemath.org/ticket/30248#comment:2
https://trac.sagemath.org/ticket/30248#comment:2
<p>
What might be causing it, is passing a generator for the inequalries (in this case you cannot get the error with the normal constructor).
</p>
<p>
If we iterate twice through it (only for number fields), the second time you obviosly don't get anything, which explains the three lines.
</p>
<p>
One solution may be to make a tuple out of the generators, once we decided what we dispose in <code>Polyhedron_base.__init__</code>. The main reason for the generators is that we do not want to generate something, which we dispose of a few lines later.
</p>
TicketmkoeppeWed, 29 Jul 2020 17:34:38 GMTpriority changed
https://trac.sagemath.org/ticket/30248#comment:3
https://trac.sagemath.org/ticket/30248#comment:3
<ul>
<li><strong>priority</strong>
changed from <em>major</em> to <em>critical</em>
</li>
</ul>
TicketjipilabThu, 30 Jul 2020 08:11:03 GMT
https://trac.sagemath.org/ticket/30248#comment:4
https://trac.sagemath.org/ticket/30248#comment:4
<p>
Found it.
</p>
<p>
Line 858 and 859 in <code>backend_normaliz</code> in the method <code>_compute_nmz_data_lists_and_field</code>, which is called by <code>_init_H_representation</code>.
</p>
<p>
The fact that the base ring is enforced to be <code>AA</code> while the data is really in <code>QQ</code> is taken care of by these 2 lines (normaliz should be used and not Qnormaliz). But! Since dilation ships 2 maps for the Hreps, the <code>data_lists</code> are empty at that point!
</p>
<p>
Ouff! Ok, I think this shouldn't be too hard to fix.
</p>
<p>
...this is a sign that this use-case was not really tested... It pops up when one wants to deal with lots of sorts of polyhedra and eventually need to fix a base ring (done in Hyperplane arrangements). Oiii.
</p>
Ticketgh-kliemThu, 30 Jul 2020 08:28:37 GMT
https://trac.sagemath.org/ticket/30248#comment:5
https://trac.sagemath.org/ticket/30248#comment:5
<p>
I implemented a few test suite methods already. If you run a test suite on your polyhedron, does it succeed?
</p>
TicketjipilabThu, 30 Jul 2020 09:07:18 GMTstatus changed; commit, branch set
https://trac.sagemath.org/ticket/30248#comment:6
https://trac.sagemath.org/ticket/30248#comment:6
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>83453fb76a747556812266da84afb98b64fc7eab</em>
</li>
<li><strong>branch</strong>
set to <em>public/30248</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=83453fb76a747556812266da84afb98b64fc7eab"><span class="icon"></span>83453fb</a></td><td><code>Fix Hrepr of normaliz for iterators</code>
</td></tr></table>
TicketjipilabThu, 30 Jul 2020 09:09:17 GMT
https://trac.sagemath.org/ticket/30248#comment:7
https://trac.sagemath.org/ticket/30248#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30248#comment:5" title="Comment 5">gh-kliem</a>:
</p>
<blockquote class="citation">
<p>
I implemented a few test suite methods already. If you run a test suite on your polyhedron, does it succeed?
</p>
</blockquote>
<p>
Yes. It seems so.
</p>
TicketjipilabThu, 30 Jul 2020 09:17:57 GMTauthor set
https://trac.sagemath.org/ticket/30248#comment:8
https://trac.sagemath.org/ticket/30248#comment:8
<ul>
<li><strong>author</strong>
set to <em>Jean-Philippe Labbé</em>
</li>
</ul>
Ticketgh-kliemThu, 30 Jul 2020 09:43:02 GMT
https://trac.sagemath.org/ticket/30248#comment:9
https://trac.sagemath.org/ticket/30248#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30248#comment:7" title="Comment 7">jipilab</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30248#comment:5" title="Comment 5">gh-kliem</a>:
</p>
<blockquote class="citation">
<p>
I implemented a few test suite methods already. If you run a test suite on your polyhedron, does it succeed?
</p>
</blockquote>
<p>
Yes. It seems so.
</p>
</blockquote>
<p>
OK. Then this is a friendly reminder to go on with it.
</p>
TicketjipilabWed, 05 Aug 2020 09:07:03 GMTcc changed
https://trac.sagemath.org/ticket/30248#comment:10
https://trac.sagemath.org/ticket/30248#comment:10
<ul>
<li><strong>cc</strong>
<em>tscrim</em> added
</li>
</ul>
<p>
Ping! I think it would be important to have this ticket in the next release...
</p>
TickettscrimWed, 05 Aug 2020 11:07:31 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/30248#comment:11
https://trac.sagemath.org/ticket/30248#comment:11
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Travis Scrimshaw</em>
</li>
</ul>
<p>
LGTM.
</p>
TicketvbraunSun, 09 Aug 2020 08:47:31 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/30248#comment:12
https://trac.sagemath.org/ticket/30248#comment:12
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>public/30248</em> to <em>83453fb76a747556812266da84afb98b64fc7eab</em>
</li>
</ul>
Ticket