Opened 6 years ago

Closed 5 years ago

#22984 closed enhancement (fixed)

Upgrade normaliz to 3.5.3 and pynormaliz to 1.12

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-8.3
Component: packages: optional Keywords: IMA-PolyGeom
Cc: Winfried Bruns, Thierry Monteil, Vincent Delecroix, Travis Scrimshaw, Jean-Philippe Labbé, Sebastian Gutsche Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw, Thierry Monteil, Jean-Philippe Labbé
Report Upstream: Completely fixed; Fix reported upstream Work issues:
Branch: 7057ef4 (Commits, GitHub, GitLab) Commit: 7057ef457e5e6f78197100f938b8dae8d8b0c5a4
Dependencies: Stopgaps:

Status badges

Change History (66)

comment:1 Changed 5 years ago by Matthias Köppe

Description: modified (diff)
Milestone: sage-8.0sage-8.1
Summary: Upgrade normaliz to 3.3.0 and pynormaliz to 1.7Upgrade normaliz to 3.4.0 and pynormaliz to 1.7

comment:2 Changed 5 years ago by Matthias Köppe

Branch: u/mkoeppe/upgrade_normaliz_to_3_4_0_and_pynormaliz_to_1_7

comment:3 Changed 5 years ago by Matthias Köppe

Cc: Thierry Monteil added
Commit: a6ccc4235fa65a26fd772ad963df01b9ece5633f
Description: modified (diff)
Summary: Upgrade normaliz to 3.4.0 and pynormaliz to 1.7Upgrade normaliz to 3.4.0 and pynormaliz to 1.7+

Tested on Mac OS X. Works well except for interrupting (may require newer pynormaliz...) and the doctest failures reported in #23586 (fixing broken pynormaliz doctests).


New commits:

be0b9d2Upgrade normaliz, pynormaliz
a6ccc42normaliz: Back out 22684_cone_reduce_memory_usage.patch

comment:4 Changed 5 years ago by Matthias Köppe

Description: modified (diff)

comment:5 Changed 5 years ago by Matthias Köppe

Authors: Matthias Koeppe

comment:6 Changed 5 years ago by git

Commit: a6ccc4235fa65a26fd772ad963df01b9ece5633f5e7a45b55eb85040b4a4c84fc0c8bd6506fdbdad

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

5e7a45bUpdate PyNormaliz to 1.8

comment:7 Changed 5 years ago by Matthias Köppe

Description: modified (diff)
Summary: Upgrade normaliz to 3.4.0 and pynormaliz to 1.7+Upgrade normaliz to 3.4.0 and pynormaliz to 1.8

comment:8 Changed 5 years ago by Matthias Köppe

Description: modified (diff)
Milestone: sage-8.1sage-8.2
Summary: Upgrade normaliz to 3.4.0 and pynormaliz to 1.8Upgrade normaliz to 3.5.1 and pynormaliz to 1.10

comment:9 Changed 5 years ago by git

Commit: 5e7a45b55eb85040b4a4c84fc0c8bd6506fdbdadf2137cc5bb49278657ce23f0f8c775400b494023

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f6011b4Upgrade normaliz to 3.5.1 and pynormaliz to 1.10
5536407normaliz: Add flint dependency
71d2194normaliz: Back out 22684_cone_reduce_memory_usage.patch
f2137ccPolyhedron_normaliz: Adjust doctest to 'uniqueness in output' of normaliz 3.5.1

comment:10 Changed 5 years ago by Matthias Köppe

Cc: Vincent Delecroix Travis Scrimshaw Jean-Philippe Labbé added
Status: newneeds_review

comment:11 Changed 5 years ago by Matthias Köppe

Description: modified (diff)

comment:12 Changed 5 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewneeds_work

I am now getting this crash (looks like an upstream problem):

sage: ieqs = [[0, 2, 0, -1, 0, 0, 0, 0, 0], [0, 0, 2, 0, -1, 0, 0, 0, 0],
....:         [1, -1, 0, 2, -1, 0, 0, 0, 0], [0, 0, -1, -1, 2, -1, 0, 0, 0],
....:         [0, 0, 0, 0, -1, 2, -1, 0, 0], [0, 0, 0, 0, 0, -1, 2, -1, 0],
....:         [0, 0, 0, 0, 0, 0, -1, 2, -1], [2, 0, 0, 0, 0, 0, 0, -1, 2],
....:         [0, -1, 0, 0, 0, 0, 0, 0, 0], [0, 0, -1, 0, 0, 0, 0, 0, 0],
....:         [0, 0, 0, -1, 0, 0, 0, 0, 0], [0, 0, 0, 0, -1, 0, 0, 0, 0],
....:         [0, 0, 0, 0, 0, -1, 0, 0, 0], [0, 0, 0, 0, 0, 0, -1, 0, 0],
....:         [0, 0, 0, 0, 0, 0, 0, -1, 0], [0, 0, 0, 0, 0, 0, 0, 0, -1],
....:         [-1, -1, -1, -1, -1, -1, -1, -1, -1], [3, 1, 0, 0, 0, 0, 0, 0, 0],
....:         [4, 0, 1, 0, 0, 0, 0, 0, 0], [6, 0, 0, 1, 0, 0, 0, 0, 0],
....:         [8, 0, 0, 0, 1, 0, 0, 0, 0], [6, 0, 0, 0, 0, 1, 0, 0, 0],
....:         [4, 0, 0, 0, 0, 0, 1, 0, 0], [2, 0, 0, 0, 0, 0, 0, 1, 0],
....:         [0, 0, 0, 0, 0, 0, 0, 0, 1]]
sage: P = Polyhedron(ieqs=ieqs, backend='normaliz')
sage: P.integral_points()
python2: ./libnormaliz/matrix.cpp:2901: libnormaliz::Matrix<Integer> libnormaliz::Matrix<Integer>::LLL_red(libnormaliz::Matrix<Integer>&, libnormaliz::Matrix<Integer>&) const [with Integer = double]: Assertion `(int) rank()==n' failed.
------------------------------------------------------------------------
/home/travis/sage-build/local/lib/python2.7/site-packages/cysignals/signals.so(+0x574b)[0x7fa5afdf274b]
/home/travis/sage-build/local/lib/python2.7/site-packages/cysignals/signals.so(+0x57b5)[0x7fa5afdf27b5]
/home/travis/sage-build/local/lib/python2.7/site-packages/cysignals/signals.so(+0x849a)[0x7fa5afdf549a]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x11390)[0x7fa5bcd37390]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0x38)[0x7fa5bc991428]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x16a)[0x7fa5bc99302a]
/lib/x86_64-linux-gnu/libc.so.6(+0x2dbd7)[0x7fa5bc989bd7]
/lib/x86_64-linux-gnu/libc.so.6(+0x2dc82)[0x7fa5bc989c82]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZNK11libnormaliz6MatrixIdE7LLL_redERS1_S2_+0x5b9)[0x7fa47314f4a9]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZNK11libnormaliz6MatrixIdE17LLL_red_transposeERS1_S2_+0x83)[0x7fa47314fdc3]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZN11libnormaliz15LLL_coordinatesIxdEENS_25Sublattice_RepresentationIT_EERKNS_6MatrixIT0_EE+0x56)[0x7fa4730cfde6]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZN11libnormaliz31LLL_coordinates_without_1st_colIxxEEvRNS_25Sublattice_RepresentationIT_EENS_6MatrixIT0_EES7_b+0x291)[0x7fa4730d0881]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZN11libnormaliz14ProjectAndLiftIxxE7computeEbb+0x194)[0x7fa473104e54]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZN11libnormaliz4ConeI10__gmp_exprIA1_12__mpz_structS3_EE16project_and_liftERNS_14ConePropertiesERNS_6MatrixIS4_EERKS9_SA_b+0x839)[0x7fa473179b69]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZN11libnormaliz4ConeI10__gmp_exprIA1_12__mpz_structS3_EE31try_approximation_or_projectionERNS_14ConePropertiesE+0xf00)[0x7fa4731f5370]
/home/travis/sage-build/local/lib/libnormaliz.so.3(_ZN11libnormaliz4ConeI10__gmp_exprIA1_12__mpz_structS3_EE7computeENS_14ConePropertiesE+0x1e1)[0x7fa4731e8671]
/home/travis/sage-build/local/lib/python2.7/site-packages/PyNormaliz_cpp.so(_Z14_NmzResultImplI10__gmp_exprIA1_12__mpz_structS2_EEP7_objectPN11libnormaliz4ConeIT_EES5_+0x75)[0x7fa4734f6cc5]
/home/travis/sage-build/local/lib/python2.7/site-packages/PyNormaliz_cpp.so(_Z10_NmzResultP7_objectS0_+0x150)[0x7fa4734ed890]

Originally found by

sage: RC = RiggedConfigurations(['E',8,1], [[3,2]])
sage: len(RC.module_generators)
python2: ./libnormaliz/matrix.cpp:2901: libnormaliz::Matrix<Integer> libnormaliz::Matrix<Integer>::LLL_red(libnormaliz::Matrix<Integer>&, libnormaliz::Matrix<Integer>&) const [with Integer = double]: Assertion `(int) rank()==n' failed.

(Mathematical note, through trying to find a smaller example, I am noticing that E8(1) is special in that it is the only place where I see non-full-dimensional non-empty polytopes in my computations. Well, I also see a 0-dimensional polytope for type D4(3), but that doesn't cause a crash as it is likely special-cased.)


Also can you update the comment before --enable-flint to also say you enable flint as it is part of the standard packages of Sage?

comment:13 Changed 5 years ago by git

Commit: f2137cc5bb49278657ce23f0f8c775400b494023b8f7abc04085155225e54ad168ce4a7884fdd6cf

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

b8f7abcnormaliz/spkg-install: Explain --enable-flint

comment:14 Changed 5 years ago by Matthias Köppe

I can confirm this crash. Here on Mac OS X, this actually becomes an unhandled SIGABRT:

Assertion failed: ((int) rank()==n), function LLL_red, file ./libnormaliz/matrix.cpp, line 2901.
------------------------------------------------------------------------
0   signals.so                          0x000000010af572d8 print_backtrace + 40
1   ???                                 0x000000010dcac1e0 0x0 + 4526359008
------------------------------------------------------------------------
Unhandled SIGABRT: An abort() occurred.
This probably occurred because a *compiled* module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.

comment:15 Changed 5 years ago by Matthias Köppe

Upstream master branch (8db5b68491a105b9185a01c29c2f126f5115e238) has different behavior but does not fix this.

comment:16 Changed 5 years ago by Thierry Monteil

Reviewers: Travis ScrimshawTravis Scrimshaw, Thierry Monteil

I confirm that the packages build and pass self-tests and doctests correctly on both 64 and 32 bit architectures on Debian stretch. I also comfirm that there is a crash on both architectures regarding the test provided by Travis (which should be added in our doctests, once the issue is fixed).

comment:17 Changed 5 years ago by Winfried Bruns

I am not sure what the matrix ieqs represents. My interpretation: inhomogeneous inequalities, and we want to compute the lattice points in the polytope they define. Under this assumption I have made a Normaliz input file rash.in

amb_space auto
inhom_inequalities
[[0, 2, 0, -1, 0, 0, 0, 0, 0],
...
[0, 0, 0, 0, 0, 0, 0, 0, 1]]
HilbertBasis

Then a run of

normaliz -c crash.in

finishes without any problems in 3.5.1 and the intended 3.5.2. It turns out that the polytope is empty:

0 module generators
1252 Hilbert basis elements of recession monoid
0 vertices of polyhedron
39 extreme rays of recession cone
11 support hyperplanes of polyhedron (homogenized)

embedding dimension = 9
affine dimension of the polyhedron = -1
rank of recession monoid = 8
...

Is my interpretation of the input correct?

comment:18 Changed 5 years ago by Winfried Bruns

Of course, if the matrix represents inhomogeneous inequalities, what is the RHS? My input above assumes it is the last coordinate. I will now try with the first.

comment:19 Changed 5 years ago by Winfried Bruns

Unfortunately Normaliz crashes when coordinate 1 is interpreted as the RHS. Will take care of it.

comment:20 Changed 5 years ago by Winfried Bruns

The problem is solved. Replace line 323 of source/libnormaliz/sublattice_representation.h

by

    if(Vertices.nr_of_rows()==0 || Vertices.rank()<EmbDim){ // use Supps for LLL coordinates

i.e., delete "-1" in this line.

comment:21 in reply to:  20 Changed 5 years ago by Travis Scrimshaw

Description: modified (diff)
Report Upstream: N/AReported upstream. Developers acknowledge bug.

Replying to Winfried:

The problem is solved. Replace line 323 of source/libnormaliz/sublattice_representation.h

by

    if(Vertices.nr_of_rows()==0 || Vertices.rank()<EmbDim){ // use Supps for LLL coordinates

i.e., delete "-1" in this line.

Great, thank you for figuring it out. I am guessing you are in the process of making a commit upstream or should I do a pull request?

comment:22 Changed 5 years ago by Travis Scrimshaw

Branch: u/mkoeppe/upgrade_normaliz_to_3_4_0_and_pynormaliz_to_1_7u/tscrim/upgrade_noramliz_pynormaliz-22984
Commit: b8f7abc04085155225e54ad168ce4a7884fdd6cff9f17a432b4d669ce88a963de93bbc2677b552be

I've pushed a branch with a fix for this problem. However, I have now come across another crash:

sage: RC = RiggedConfigurations(['E',8,1], [[4,2]])
sage: len(RC.module_generators)
python2: ./libnormaliz/project_and_lift.cpp:75: std::vector<long unsigned int> libnormaliz::ProjectAndLift<IntegerPL, IntegerRet>::order_supps(const libnormaliz::Matrix<Integer>&) [with IntegerPL = long long int; IntegerRet = long long int]: Assertion `Supps.nr_of_rows()>0' failed.
------------------------------------------------------------------------

I have verified that this did at least not crash before (well, up to my patience level of 2 minutes).

However, much more worrying is this:

sage: RC = RiggedConfigurations(['E',8,1], [[5,2]])
sage: %time len(RC.module_generators)
CPU times: user 25 s, sys: 52 ms, total: 25 s
Wall time: 6.95 s
6009

versus before

sage: RC = RiggedConfigurations(['E',8,1], [[5,2]])
sage: %time len(RC.module_generators)
CPU times: user 1min 11s, sys: 320 ms, total: 1min 11s
Wall time: 14.6 s
7066

Although the [5,1] example both returns 75.

I will extract the polytopes and find out which one gives a different number of lattice points. Give me a few minutes.


New commits:

f9f17a4Added patch to fix upstream issue #176.

comment:23 Changed 5 years ago by Travis Scrimshaw

Okay, here is the [4,1] computation to completion:

sage: RC = RiggedConfigurations(['E',8,1], [[4,2]])
sage: %time len(RC.module_generators)
CPU times: user 19min 37s, sys: 5.2 s, total: 19min 42s
Wall time: 4min 37s
288299

comment:24 Changed 5 years ago by Travis Scrimshaw

Okay, here is at least the first polytope that goes wrong:

sage: ieqs = [[0, 2, 0, -1, 0, 0, 0, 0, 0],
....:         [0, 0, 2, 0, -1, 0, 0, 0, 0],
....:         [1, -1, 0, 2, -1, 0, 0, 0, 0],
....:         [0, 0, -1, -1, 2, -1, 0, 0, 0],
....:         [0, 0, 0, 0, -1, 2, -1, 0, 0],
....:         [0, 0, 0, 0, 0, -1, 2, -1, 0],
....:         [1, 0, 0, 0, 0, 0, -1, 2, -1],
....:         [0, 0, 0, 0, 0, 0, 0, -1, 2],
....:         [0, -1, 0, 0, 0, 0, 0, 0, 0],
....:         [0, 0, -1, 0, 0, 0, 0, 0, 0],
....:         [0, 0, 0, -1, 0, 0, 0, 0, 0],
....:         [0, 0, 0, 0, -1, 0, 0, 0, 0],
....:         [0, 0, 0, 0, 0, -1, 0, 0, 0],
....:         [0, 0, 0, 0, 0, 0, -1, 0, 0],
....:         [0, 0, 0, 0, 0, 0, 0, -1, 0],
....:         [0, 0, 0, 0, 0, 0, 0, 0, -1],
....:         [-1, -1, -1, -1, -1, -1, -1, -1, -1],
....:         [2, 1, 0, 0, 0, 0, 0, 0, 0],
....:         [4, 0, 1, 0, 0, 0, 0, 0, 0],
....:         [4, 0, 0, 1, 0, 0, 0, 0, 0],
....:         [7, 0, 0, 0, 1, 0, 0, 0, 0],
....:         [6, 0, 0, 0, 0, 1, 0, 0, 0],
....:         [4, 0, 0, 0, 0, 0, 1, 0, 0],
....:         [2, 0, 0, 0, 0, 0, 0, 1, 0],
....:         [1, 0, 0, 0, 0, 0, 0, 0, 1]]
sage: P = Polyhedron(ieqs=ieqs, backend='normaliz')
sage: P.integral_points()
((-2, -2, -4, -5, -4, -3, -2, -1),
 (-2, -2, -4, -5, -4, -3, -2, 0),
 (-1, -2, -3, -4, -3, -2, -2, -1),
 (-1, -2, -3, -4, -3, -2, -1, 0),
 (-1, -1, -2, -2, -2, -2, -2, -1),
 (-1, -1, -2, -2, -1, -1, -1, 0),
 (-1, -1, -2, -2, -1, 0, 0, 0),
 (-1, 0, -2, -2, -2, -2, -2, -1),
 (0, -1, -1, -2, -2, -2, -2, -1),
 (0, 0, -1, -1, -1, -1, -1, 0))
sage: P
A 8-dimensional polyhedron in QQ^8 defined as the convex hull of 134 vertices

versus the new version with the patch:

sage: P.integral_points()
((-1, -2, -3, -4, -3, -2, -1, 0), (0, -1, -1, -2, -2, -2, -2, -1))

comment:25 Changed 5 years ago by Travis Scrimshaw

Here's also the example that causes the new crash:

sage: ieqs = [[0, 2, 0, -1, 0, 0, 0, 0, 0],
....:         [0, 0, 2, 0, -1, 0, 0, 0, 0],
....:         [0, -1, 0, 2, -1, 0, 0, 0, 0],
....:         [1, 0, -1, -1, 2, -1, 0, 0, 0],
....:         [0, 0, 0, 0, -1, 2, -1, 0, 0],
....:         [0, 0, 0, 0, 0, -1, 2, -1, 0],
....:         [1, 0, 0, 0, 0, 0, -1, 2, -1],
....:         [0, 0, 0, 0, 0, 0, 0, -1, 2],
....:         [0, -1, 0, 0, 0, 0, 0, 0, 0],
....:         [0, 0, -1, 0, 0, 0, 0, 0, 0],
....:         [0, 0, 0, -1, 0, 0, 0, 0, 0],
....:         [0, 0, 0, 0, -1, 0, 0, 0, 0],
....:         [0, 0, 0, 0, 0, -1, 0, 0, 0],
....:         [0, 0, 0, 0, 0, 0, -1, 0, 0],
....:         [0, 0, 0, 0, 0, 0, 0, -1, 0],
....:         [0, 0, 0, 0, 0, 0, 0, 0, -1],
....:         [-1, -1, -1, -1, -1, -1, -1, -1, -1],
....:         [3, 1, 0, 0, 0, 0, 0, 0, 0],
....:         [4, 0, 1, 0, 0, 0, 0, 0, 0],
....:         [6, 0, 0, 1, 0, 0, 0, 0, 0],
....:         [8, 0, 0, 0, 1, 0, 0, 0, 0],
....:         [6, 0, 0, 0, 0, 1, 0, 0, 0],
....:         [4, 0, 0, 0, 0, 0, 1, 0, 0],
....:         [2, 0, 0, 0, 0, 0, 0, 1, 0],
....:         [1, 0, 0, 0, 0, 0, 0, 0, 1]]
sage: P = Polyhedron(ieqs=ieqs, backend='normaliz')
sage: P.integral_points()
python2: ./libnormaliz/project_and_lift.cpp:75: std::vector<long unsigned int> libnormaliz::ProjectAndLift<IntegerPL, IntegerRet>::order_supps(const libnormaliz::Matrix<Integer>&) [with IntegerPL = long long int; IntegerRet = long long int]: Assertion `Supps.nr_of_rows()>0' failed.
------------------------------------------------------------------------

comment:26 Changed 5 years ago by Matthias Köppe

A hint: If you use P = Polyhedron(ieqs=ieqs, backend='normaliz', verbose=True), the PyNormaliz invocations are shown, making it easier for upstream

comment:27 Changed 5 years ago by Travis Scrimshaw

I wasn't aware of that, thank you. Although it does not yield much more information on the new crash:

sage: P = Polyhedron(ieqs=ieqs, backend='normaliz', verbose=True)
# Calling PyNormaliz.NmzCone(['inhom_equations', [], 'inhom_inequalities',
 [[2, 0, -1, 0, 0, 0, 0, 0, 0], [0, 2, 0, -1, 0, 0, 0, 0, 0],
 [-1, 0, 2, -1, 0, 0, 0, 0, 0], [0, -1, -1, 2, -1, 0, 0, 0, 1],
 [0, 0, 0, -1, 2, -1, 0, 0, 0], [0, 0, 0, 0, -1, 2, -1, 0, 0],
 [0, 0, 0, 0, 0, -1, 2, -1, 1], [0, 0, 0, 0, 0, 0, -1, 2, 0],
 [-1, 0, 0, 0, 0, 0, 0, 0, 0], [0, -1, 0, 0, 0, 0, 0, 0, 0],
 [0, 0, -1, 0, 0, 0, 0, 0, 0], [0, 0, 0, -1, 0, 0, 0, 0, 0],
 [0, 0, 0, 0, -1, 0, 0, 0, 0], [0, 0, 0, 0, 0, -1, 0, 0, 0],
 [0, 0, 0, 0, 0, 0, -1, 0, 0], [0, 0, 0, 0, 0, 0, 0, -1, 0],
 [-1, -1, -1, -1, -1, -1, -1, -1, -1], [1, 0, 0, 0, 0, 0, 0, 0, 3],
 [0, 1, 0, 0, 0, 0, 0, 0, 4], [0, 0, 1, 0, 0, 0, 0, 0, 6],
 [0, 0, 0, 1, 0, 0, 0, 0, 8], [0, 0, 0, 0, 1, 0, 0, 0, 6],
 [0, 0, 0, 0, 0, 1, 0, 0, 4], [0, 0, 0, 0, 0, 0, 1, 0, 2],
 [0, 0, 0, 0, 0, 0, 0, 1, 1]]])
sage: P.integral_points()
python2: ./libnormaliz/project_and_lift.cpp:75: std::vector<long unsigned int> libnormaliz::ProjectAndLift<IntegerPL, IntegerRet>::order_supps(const libnormaliz::Matrix<Integer>&) [with IntegerPL = long long int; IntegerRet = long long int]: Assertion `Supps.nr_of_rows()>0' failed.
------------------------------------------------------------------------

Here is also the output for the lattice point computation:

sage: P = Polyhedron(ieqs=ieqs, backend='normaliz', verbose=True)
# Calling PyNormaliz.NmzCone(['inhom_equations', [], 'inhom_inequalities',
 [[2, 0, -1, 0, 0, 0, 0, 0, 0], [0, 2, 0, -1, 0, 0, 0, 0, 0],
 [-1, 0, 2, -1, 0, 0, 0, 0, 1], [0, -1, -1, 2, -1, 0, 0, 0, 0],
 [0, 0, 0, -1, 2, -1, 0, 0, 0], [0, 0, 0, 0, -1, 2, -1, 0, 0],
 [0, 0, 0, 0, 0, -1, 2, -1, 1], [0, 0, 0, 0, 0, 0, -1, 2, 0],
 [-1, 0, 0, 0, 0, 0, 0, 0, 0], [0, -1, 0, 0, 0, 0, 0, 0, 0],
 [0, 0, -1, 0, 0, 0, 0, 0, 0], [0, 0, 0, -1, 0, 0, 0, 0, 0],
 [0, 0, 0, 0, -1, 0, 0, 0, 0], [0, 0, 0, 0, 0, -1, 0, 0, 0],
 [0, 0, 0, 0, 0, 0, -1, 0, 0], [0, 0, 0, 0, 0, 0, 0, -1, 0],
 [-1, -1, -1, -1, -1, -1, -1, -1, -1], [1, 0, 0, 0, 0, 0, 0, 0, 2],
 [0, 1, 0, 0, 0, 0, 0, 0, 4], [0, 0, 1, 0, 0, 0, 0, 0, 4],
 [0, 0, 0, 1, 0, 0, 0, 0, 7], [0, 0, 0, 0, 1, 0, 0, 0, 6],
 [0, 0, 0, 0, 0, 1, 0, 0, 4], [0, 0, 0, 0, 0, 0, 1, 0, 2],
 [0, 0, 0, 0, 0, 0, 0, 1, 1]]])
sage: P.integral_points()
((-1, -2, -3, -4, -3, -2, -1, 0), (0, -1, -1, -2, -2, -2, -2, -1))
Last edited 5 years ago by Travis Scrimshaw (previous) (diff)

comment:28 Changed 5 years ago by Travis Scrimshaw

PS - That means the docstring for Polyhedron needs a minor adjustment:

    - ``verbose`` -- boolean (default: ``False``). Whether to print
      verbose output for debugging purposes. Only supported by the cdd
      backends.

comment:29 Changed 5 years ago by Winfried Bruns

The wrong number of lattice points is a more severe problem. I don't have an explanation for it.

The (first and solved) problem in sublattice_representation.h has nothing to do with it. After the change in sublattice_representation.h:323 a second problem has become visible there in Normaliz' own tests. One must change line 325 to

        if(SuppHelp.rank()<EmbDim-1)

But againthis has nothing to do with the wrong number of lattice points.

comment:30 Changed 5 years ago by git

Commit: f9f17a432b4d669ce88a963de93bbc2677b552bed3f7ceb711bc98ae06bf4d9a921ee376c09c3fda

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

d3f7cebNew version of patch to fix next crash.

comment:31 in reply to:  29 Changed 5 years ago by Travis Scrimshaw

Replying to Winfried:

The wrong number of lattice points is a more severe problem. I don't have an explanation for it.

I am afraid all I can do is give you examples from my use-case as I do not understand too much about how Normaliz works. Thank you for your continued efforts and quick responses.

The (first and solved) problem in sublattice_representation.h has nothing to do with it. After the change in sublattice_representation.h:323 a second problem has become visible there in Normaliz' own tests. One must change line 325 to

        if(SuppHelp.rank()<EmbDim-1)

But again this has nothing to do with the wrong number of lattice points.

I added that to the patch, but (as you might be aware) I am now getting yet another crash with the same example as in comment:25:

sage: P = Polyhedron(ieqs=ieqs, backend='normaliz', verbose=True)
# Calling PyNormaliz.NmzCone(['inhom_equations', [], 'inhom_inequalities',
 [[2, 0, -1, 0, 0, 0, 0, 0, 0], [0, 2, 0, -1, 0, 0, 0, 0, 0],
 [-1, 0, 2, -1, 0, 0, 0, 0, 0], [0, -1, -1, 2, -1, 0, 0, 0, 1],
 [0, 0, 0, -1, 2, -1, 0, 0, 0], [0, 0, 0, 0, -1, 2, -1, 0, 0],
 [0, 0, 0, 0, 0, -1, 2, -1, 1], [0, 0, 0, 0, 0, 0, -1, 2, 0],
 [-1, 0, 0, 0, 0, 0, 0, 0, 0], [0, -1, 0, 0, 0, 0, 0, 0, 0],
 [0, 0, -1, 0, 0, 0, 0, 0, 0], [0, 0, 0, -1, 0, 0, 0, 0, 0],
 [0, 0, 0, 0, -1, 0, 0, 0, 0], [0, 0, 0, 0, 0, -1, 0, 0, 0],
 [0, 0, 0, 0, 0, 0, -1, 0, 0], [0, 0, 0, 0, 0, 0, 0, -1, 0],
 [-1, -1, -1, -1, -1, -1, -1, -1, -1], [1, 0, 0, 0, 0, 0, 0, 0, 3],
 [0, 1, 0, 0, 0, 0, 0, 0, 4], [0, 0, 1, 0, 0, 0, 0, 0, 6],
 [0, 0, 0, 1, 0, 0, 0, 0, 8], [0, 0, 0, 0, 1, 0, 0, 0, 6],
 [0, 0, 0, 0, 0, 1, 0, 0, 4], [0, 0, 0, 0, 0, 0, 1, 0, 2],
 [0, 0, 0, 0, 0, 0, 0, 1, 1]]])
sage: P
A 8-dimensional polyhedron in QQ^8 defined as the convex hull of 132 vertices
sage: P.integral_points()
python2: ./libnormaliz/project_and_lift.cpp:75: std::vector<long unsigned int> libnormaliz::ProjectAndLift<IntegerPL, IntegerRet>::order_supps(const libnormaliz::Matrix<Integer>&) [with IntegerPL = long long int; IntegerRet = long long int]: Assertion `Supps.nr_of_rows()>0' failed.
------------------------------------------------------------------------

Addendum - It is the same error as before.

Last edited 5 years ago by Travis Scrimshaw (previous) (diff)

comment:32 Changed 5 years ago by Winfried Bruns

I think all the observed problems are solved. More tomorrow.

comment:33 Changed 5 years ago by Travis Scrimshaw

Great, thank you!

comment:34 Changed 5 years ago by Winfried Bruns

The problem with the wrong number of lattice points (or a failing assertion) was not caused by a severe problem, but by a silly bug. As a temporary fix I suggest to include as lines cone.cpp:3804 and 3805

    if(!is_parallelotope)
        Pair.clear();

So with some surrounding the code there should be

            is_Computed.set(ConeProperty::IsPointed);
        }
    }
    
    if(!is_parallelotope)
        Pair.clear();
    
    if(!inhomogeneous && !isComputed(ConeProperty::Grading))
        return;

I plan to release 3.5.2 at the end of this week. It will contain a more conceptual solution.

Please test.

comment:35 Changed 5 years ago by git

Commit: d3f7ceb711bc98ae06bf4d9a921ee376c09c3fda737552d95ff6d810c56611560fa9b62f888cdafc

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

737552dUpdating patch with upstream fix for wrong number of lattice points.

comment:36 Changed 5 years ago by Travis Scrimshaw

Thank you. I have verified that I get again

sage: RC = RiggedConfigurations(['E',8,1], [[5,2]])
sage: %time len(RC.module_generators)
CPU times: user 27.6 s, sys: 68 ms, total: 27.7 s
Wall time: 7.8 s
7066
sage: RC = RiggedConfigurations(['E',8,1], [[4,2]])
sage: %time len(RC.module_generators)
CPU times: user 5min 6s, sys: 944 ms, total: 5min 7s
Wall time: 1min 36s
288299

and the >2x speedup is very impressive. :) I've updated the patch accordingly.

@mkoeppe Perhaps we should just wait on this upgrade until 3.5.2 is released to avoid the patch since it will be in just a few days?

As far as adding tests go, I think we should explicitly add the polytopes mentioned above rather than the RC code. They are isolated and much faster to test.

comment:37 in reply to:  36 Changed 5 years ago by Matthias Köppe

Replying to tscrim:

@mkoeppe Perhaps we should just wait on this upgrade until 3.5.2 is released to avoid the patch since it will be in just a few days?

Yes, I think so too.

As far as adding tests go, I think we should explicitly add the polytopes mentioned above rather than the RC code. They are isolated and much faster to test.

I agree.

comment:38 Changed 5 years ago by git

Commit: 737552d95ff6d810c56611560fa9b62f888cdafc57d40b030deb164404f6a245af0c3734ebf1eb40

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

530302dMerge branch 'u/tscrim/upgrade_noramliz_pynormaliz-22984' of git://trac.sagemath.org/sage into u/tscrim/upgrade_noramliz_pynormaliz-22984
57d40b0Upgrade Normaliz to 3.5.2.

comment:39 Changed 5 years ago by Travis Scrimshaw

Description: modified (diff)
Report Upstream: Reported upstream. Developers acknowledge bug.Completely fixed; Fix reported upstream
Status: needs_workneeds_info

Here is with 3.5.2 and the tests I did above now work. Where would a good place be to add the doctests?

comment:40 Changed 5 years ago by Matthias Köppe

Status: needs_infoneeds_work
Summary: Upgrade normaliz to 3.5.1 and pynormaliz to 1.10Upgrade normaliz to 3.5.2 and pynormaliz to 1.10

I would think the doctests should go to Polyhedron_normaliz.integral_points.

comment:41 Changed 5 years ago by git

Commit: 57d40b030deb164404f6a245af0c3734ebf1eb406ba442d0586b9fdedec07a2e8f50bcd5d1a4a913

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

6ba442dAdding tests from comment:24,25 of #22984.

comment:42 Changed 5 years ago by Travis Scrimshaw

Status: needs_workneeds_review

Okay, done.

comment:43 Changed 5 years ago by Travis Scrimshaw

ping

comment:44 Changed 5 years ago by Matthias Köppe

Description: modified (diff)
Status: needs_reviewneeds_work
Summary: Upgrade normaliz to 3.5.2 and pynormaliz to 1.10Upgrade normaliz to 3.5.2 and pynormaliz to 1.12
  1. Gutsche informs us that we should upgrade pynormaliz to 1.12

comment:45 Changed 5 years ago by Matthias Köppe

Cc: Sebastian Gutsche added

comment:46 Changed 5 years ago by Jean-Philippe Labbé

Reviewers: Travis Scrimshaw, Thierry MonteilTravis Scrimshaw, Thierry Monteil, Jean-Philippe Labbé

All tests passed with sage8.2.beta8.

@Travis: Do you still want to add a comment about the verbose of Polyhedron?

Otherwise, it seems that we need to wait for 1.12 of pynormaliz check everything one last time.

comment:47 Changed 5 years ago by Matthias Köppe

Branch: u/tscrim/upgrade_noramliz_pynormaliz-22984u/mkoeppe/upgrade_noramliz_pynormaliz-22984

comment:48 Changed 5 years ago by Matthias Köppe

Commit: 6ba442d0586b9fdedec07a2e8f50bcd5d1a4a913183087a21eaaf64b464fa6ffe0aaf94819d6626c
Status: needs_workneeds_review

New commits:

183087aUpgrade PyNormaliz to 1.12

comment:49 Changed 5 years ago by Jean-Philippe Labbé

Branch: u/mkoeppe/upgrade_noramliz_pynormaliz-22984u/jipilab/upgrade_normaliz_pynormaliz-22984
Commit: 183087a21eaaf64b464fa6ffe0aaf94819d6626c7f47ec9f71498f744614e28ed8316b72b1904bdd
Status: needs_reviewpositive_review

New commits:

7f47ec9Adapted the polyhedron docstring

comment:50 Changed 5 years ago by Jean-Philippe Labbé

It looks good to me now and all tests pass in the polyhedron folder.

comment:51 Changed 5 years ago by Matthias Köppe

Status: positive_reviewneeds_work

Compilation errors with a specific gcc version with PyNormaliz?. Fixed in normaliz 3.5.3

comment:52 Changed 5 years ago by Matthias Köppe

Description: modified (diff)
Summary: Upgrade normaliz to 3.5.2 and pynormaliz to 1.12Upgrade normaliz to 3.5.3 and pynormaliz to 1.12

comment:53 Changed 5 years ago by Matthias Köppe

Branch: u/jipilab/upgrade_normaliz_pynormaliz-22984u/mkoeppe/upgrade_normaliz_pynormaliz-22984

comment:54 Changed 5 years ago by Matthias Köppe

Commit: 7f47ec9f71498f744614e28ed8316b72b1904bdd117e428c112921f5a1d555daf3a7b0ed10828532
Status: needs_workneeds_review

New commits:

117e428Upgrade normaliz to 3.5.3

comment:55 Changed 5 years ago by Jean-Philippe Labbé

Install the latest normaliz and got:

~/sage/src/sage/geometry/polyhedron$ sage -t *.py
Using --optional=4ti2,bliss,latte_int,lidia,lrslib,mpir,normaliz,pynormaliz,python2,sage
Doctesting 25 files.
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------

comment:56 Changed 5 years ago by Jean-Philippe Labbé

Status: needs_reviewpositive_review

comment:57 in reply to:  46 ; Changed 5 years ago by Travis Scrimshaw

Replying to jipilab:

@Travis: Do you still want to add a comment about the verbose of Polyhedron?

It can be on a separate ticket, but the docstring for verbose should also say it is supported for the normaliz backend as well.

comment:58 in reply to:  57 Changed 5 years ago by Jean-Philippe Labbé

Replying to tscrim:

Replying to jipilab:

@Travis: Do you still want to add a comment about the verbose of Polyhedron?

It can be on a separate ticket, but the docstring for verbose should also say it is supported for the normaliz backend as well.

I went ahead and made the change already... I realized it was not a big deal.

comment:59 Changed 5 years ago by Travis Scrimshaw

Ah, thanks. I didn't look at the diffs.

comment:60 Changed 5 years ago by Jean-Philippe Labbé

Branch: u/mkoeppe/upgrade_normaliz_pynormaliz-22984u/jipilab/upgrade_normaliz_pynormaliz-22984
Commit: 117e428c112921f5a1d555daf3a7b0ed108285327057ef457e5e6f78197100f938b8dae8d8b0c5a4

Merged the forgotten commit.


New commits:

7f47ec9Adapted the polyhedron docstring
7057ef4Merge branch to get docstring adaptation

comment:61 Changed 5 years ago by Jean-Philippe Labbé

Status: positive_reviewneeds_review

comment:62 Changed 5 years ago by Matthias Köppe

Status: needs_reviewpositive_review

comment:63 Changed 5 years ago by Matthias Köppe

Keywords: IMA-PolyGeom added

comment:64 Changed 5 years ago by Matthias Köppe

Description: modified (diff)

comment:65 Changed 5 years ago by Jean-Philippe Labbé

Milestone: sage-8.2sage-8.3

comment:66 Changed 5 years ago by Volker Braun

Branch: u/jipilab/upgrade_normaliz_pynormaliz-229847057ef457e5e6f78197100f938b8dae8d8b0c5a4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.