Opened 3 years ago
Closed 2 years ago
#22984 closed enhancement (fixed)
Upgrade normaliz to 3.5.3 and pynormaliz to 1.12
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  packages: optional  Keywords:  IMAPolyGeom 
Cc:  Winfried, tmonteil, vdelecroix, tscrim, jipilab, ghsebasguts  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Travis Scrimshaw, Thierry Monteil, JeanPhilippe Labbé 
Report Upstream:  Completely fixed; Fix reported upstream  Work issues:  
Branch:  7057ef4 (Commits)  Commit:  7057ef457e5e6f78197100f938b8dae8d8b0c5a4 
Dependencies:  Stopgaps: 
Description (last modified by )
Among many new features and improvements, these new versions make computations interruptible with ^C
.
Source tarballs:
 https://github.com/Normaliz/Normaliz/releases/download/v3.5.3/normaliz3.5.3.tar.gz
 https://pypi.python.org/packages/30/48/d2ad6e4defcce3959c66ee541cf5c0eda12186e42ecd68a0531813a16bd0/PyNormaliz1.12.tar.gz#md5=1ec3419eaa061a0ee4b4f1ffca9cb44d
Upstream bug: https://github.com/Normaliz/Normaliz/issues/176
Followup ticket with more cuttingedge PyNormaliz
version: #25090.
Change History (66)
comment:1 Changed 3 years ago by
 Description modified (diff)
 Milestone changed from sage8.0 to sage8.1
 Summary changed from Upgrade normaliz to 3.3.0 and pynormaliz to 1.7 to Upgrade normaliz to 3.4.0 and pynormaliz to 1.7
comment:2 Changed 3 years ago by
 Branch set to u/mkoeppe/upgrade_normaliz_to_3_4_0_and_pynormaliz_to_1_7
comment:3 Changed 3 years ago by
 Cc tmonteil added
 Commit set to a6ccc4235fa65a26fd772ad963df01b9ece5633f
 Description modified (diff)
 Summary changed from Upgrade normaliz to 3.4.0 and pynormaliz to 1.7 to Upgrade normaliz to 3.4.0 and pynormaliz to 1.7+
comment:4 Changed 3 years ago by
 Description modified (diff)
comment:5 Changed 3 years ago by
comment:6 Changed 3 years ago by
 Commit changed from a6ccc4235fa65a26fd772ad963df01b9ece5633f to 5e7a45b55eb85040b4a4c84fc0c8bd6506fdbdad
Branch pushed to git repo; I updated commit sha1. New commits:
5e7a45b  Update PyNormaliz to 1.8

comment:7 Changed 3 years ago by
 Description modified (diff)
 Summary changed from Upgrade normaliz to 3.4.0 and pynormaliz to 1.7+ to Upgrade normaliz to 3.4.0 and pynormaliz to 1.8
comment:8 Changed 2 years ago by
 Description modified (diff)
 Milestone changed from sage8.1 to sage8.2
 Summary changed from Upgrade normaliz to 3.4.0 and pynormaliz to 1.8 to Upgrade normaliz to 3.5.1 and pynormaliz to 1.10
comment:9 Changed 2 years ago by
 Commit changed from 5e7a45b55eb85040b4a4c84fc0c8bd6506fdbdad to f2137cc5bb49278657ce23f0f8c775400b494023
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f6011b4  Upgrade normaliz to 3.5.1 and pynormaliz to 1.10

5536407  normaliz: Add flint dependency

71d2194  normaliz: Back out 22684_cone_reduce_memory_usage.patch

f2137cc  Polyhedron_normaliz: Adjust doctest to 'uniqueness in output' of normaliz 3.5.1

comment:10 Changed 2 years ago by
 Cc vdelecroix tscrim jipilab added
 Status changed from new to needs_review
comment:11 Changed 2 years ago by
 Description modified (diff)
comment:12 Changed 2 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to needs_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/sagebuild/local/lib/python2.7/sitepackages/cysignals/signals.so(+0x574b)[0x7fa5afdf274b] /home/travis/sagebuild/local/lib/python2.7/sitepackages/cysignals/signals.so(+0x57b5)[0x7fa5afdf27b5] /home/travis/sagebuild/local/lib/python2.7/sitepackages/cysignals/signals.so(+0x849a)[0x7fa5afdf549a] /lib/x86_64linuxgnu/libpthread.so.0(+0x11390)[0x7fa5bcd37390] /lib/x86_64linuxgnu/libc.so.6(gsignal+0x38)[0x7fa5bc991428] /lib/x86_64linuxgnu/libc.so.6(abort+0x16a)[0x7fa5bc99302a] /lib/x86_64linuxgnu/libc.so.6(+0x2dbd7)[0x7fa5bc989bd7] /lib/x86_64linuxgnu/libc.so.6(+0x2dc82)[0x7fa5bc989c82] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZNK11libnormaliz6MatrixIdE7LLL_redERS1_S2_+0x5b9)[0x7fa47314f4a9] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZNK11libnormaliz6MatrixIdE17LLL_red_transposeERS1_S2_+0x83)[0x7fa47314fdc3] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZN11libnormaliz15LLL_coordinatesIxdEENS_25Sublattice_RepresentationIT_EERKNS_6MatrixIT0_EE+0x56)[0x7fa4730cfde6] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZN11libnormaliz31LLL_coordinates_without_1st_colIxxEEvRNS_25Sublattice_RepresentationIT_EENS_6MatrixIT0_EES7_b+0x291)[0x7fa4730d0881] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZN11libnormaliz14ProjectAndLiftIxxE7computeEbb+0x194)[0x7fa473104e54] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZN11libnormaliz4ConeI10__gmp_exprIA1_12__mpz_structS3_EE16project_and_liftERNS_14ConePropertiesERNS_6MatrixIS4_EERKS9_SA_b+0x839)[0x7fa473179b69] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZN11libnormaliz4ConeI10__gmp_exprIA1_12__mpz_structS3_EE31try_approximation_or_projectionERNS_14ConePropertiesE+0xf00)[0x7fa4731f5370] /home/travis/sagebuild/local/lib/libnormaliz.so.3(_ZN11libnormaliz4ConeI10__gmp_exprIA1_12__mpz_structS3_EE7computeENS_14ConePropertiesE+0x1e1)[0x7fa4731e8671] /home/travis/sagebuild/local/lib/python2.7/sitepackages/PyNormaliz_cpp.so(_Z14_NmzResultImplI10__gmp_exprIA1_12__mpz_structS2_EEP7_objectPN11libnormaliz4ConeIT_EES5_+0x75)[0x7fa4734f6cc5] /home/travis/sagebuild/local/lib/python2.7/sitepackages/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 E_{8}^{(1)} is special in that it is the only place where I see nonfulldimensional nonempty polytopes in my computations. Well, I also see a 0dimensional polytope for type D_{4}^{(3)}, but that doesn't cause a crash as it is likely specialcased.)
Also can you update the comment before enableflint
to also say you enable flint as it is part of the standard packages of Sage?
comment:13 Changed 2 years ago by
 Commit changed from f2137cc5bb49278657ce23f0f8c775400b494023 to b8f7abc04085155225e54ad168ce4a7884fdd6cf
Branch pushed to git repo; I updated commit sha1. New commits:
b8f7abc  normaliz/spkginstall: Explain enableflint

comment:14 Changed 2 years ago by
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 2 years ago by
Upstream master branch (8db5b68491a105b9185a01c29c2f126f5115e238) has different behavior but does not fix this.
comment:16 Changed 2 years ago by
 Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Thierry Monteil
I confirm that the packages build and pass selftests 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 2 years ago by
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 2 years ago by
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 2 years ago by
Unfortunately Normaliz crashes when coordinate 1 is interpreted as the RHS. Will take care of it.
comment:20 followup: ↓ 21 Changed 2 years ago by
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 2 years ago by
 Description modified (diff)
 Report Upstream changed from N/A to Reported 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 coordinatesi.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 2 years ago by
 Branch changed from u/mkoeppe/upgrade_normaliz_to_3_4_0_and_pynormaliz_to_1_7 to u/tscrim/upgrade_noramliz_pynormaliz22984
 Commit changed from b8f7abc04085155225e54ad168ce4a7884fdd6cf to f9f17a432b4d669ce88a963de93bbc2677b552be
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:
f9f17a4  Added patch to fix upstream issue #176.

comment:23 Changed 2 years ago by
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 2 years ago by
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 8dimensional 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 2 years ago by
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 2 years ago by
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 2 years ago by
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))
comment:28 Changed 2 years ago by
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 followup: ↓ 31 Changed 2 years ago by
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()<EmbDim1)
But againthis has nothing to do with the wrong number of lattice points.
comment:30 Changed 2 years ago by
 Commit changed from f9f17a432b4d669ce88a963de93bbc2677b552be to d3f7ceb711bc98ae06bf4d9a921ee376c09c3fda
Branch pushed to git repo; I updated commit sha1. New commits:
d3f7ceb  New version of patch to fix next crash.

comment:31 in reply to: ↑ 29 Changed 2 years ago by
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 usecase 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()<EmbDim1)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 8dimensional 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.
comment:32 Changed 2 years ago by
I think all the observed problems are solved. More tomorrow.
comment:33 Changed 2 years ago by
Great, thank you!
comment:34 Changed 2 years ago by
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 2 years ago by
 Commit changed from d3f7ceb711bc98ae06bf4d9a921ee376c09c3fda to 737552d95ff6d810c56611560fa9b62f888cdafc
Branch pushed to git repo; I updated commit sha1. New commits:
737552d  Updating patch with upstream fix for wrong number of lattice points.

comment:36 followup: ↓ 37 Changed 2 years ago by
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 2 years ago by
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 2 years ago by
 Commit changed from 737552d95ff6d810c56611560fa9b62f888cdafc to 57d40b030deb164404f6a245af0c3734ebf1eb40
comment:39 Changed 2 years ago by
 Description modified (diff)
 Report Upstream changed from Reported upstream. Developers acknowledge bug. to Completely fixed; Fix reported upstream
 Status changed from needs_work to needs_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 2 years ago by
 Status changed from needs_info to needs_work
 Summary changed from Upgrade normaliz to 3.5.1 and pynormaliz to 1.10 to Upgrade 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 2 years ago by
 Commit changed from 57d40b030deb164404f6a245af0c3734ebf1eb40 to 6ba442d0586b9fdedec07a2e8f50bcd5d1a4a913
Branch pushed to git repo; I updated commit sha1. New commits:
6ba442d  Adding tests from comment:24,25 of #22984.

comment:43 Changed 2 years ago by
ping
comment:44 Changed 2 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
 Summary changed from Upgrade normaliz to 3.5.2 and pynormaliz to 1.10 to Upgrade normaliz to 3.5.2 and pynormaliz to 1.12
 Gutsche informs us that we should upgrade pynormaliz to 1.12
comment:45 Changed 2 years ago by
 Cc ghsebasguts added
comment:46 followup: ↓ 57 Changed 2 years ago by
 Reviewers changed from Travis Scrimshaw, Thierry Monteil to Travis Scrimshaw, Thierry Monteil, JeanPhilippe 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 2 years ago by
 Branch changed from u/tscrim/upgrade_noramliz_pynormaliz22984 to u/mkoeppe/upgrade_noramliz_pynormaliz22984
comment:48 Changed 2 years ago by
 Commit changed from 6ba442d0586b9fdedec07a2e8f50bcd5d1a4a913 to 183087a21eaaf64b464fa6ffe0aaf94819d6626c
 Status changed from needs_work to needs_review
New commits:
183087a  Upgrade PyNormaliz to 1.12

comment:49 Changed 2 years ago by
 Branch changed from u/mkoeppe/upgrade_noramliz_pynormaliz22984 to u/jipilab/upgrade_normaliz_pynormaliz22984
 Commit changed from 183087a21eaaf64b464fa6ffe0aaf94819d6626c to 7f47ec9f71498f744614e28ed8316b72b1904bdd
 Status changed from needs_review to positive_review
New commits:
7f47ec9  Adapted the polyhedron docstring

comment:50 Changed 2 years ago by
It looks good to me now and all tests pass in the polyhedron folder.
comment:51 Changed 2 years ago by
 Status changed from positive_review to needs_work
Compilation errors with a specific gcc version with PyNormaliz?. Fixed in normaliz 3.5.3
comment:52 Changed 2 years ago by
 Description modified (diff)
 Summary changed from Upgrade normaliz to 3.5.2 and pynormaliz to 1.12 to Upgrade normaliz to 3.5.3 and pynormaliz to 1.12
comment:53 Changed 2 years ago by
 Branch changed from u/jipilab/upgrade_normaliz_pynormaliz22984 to u/mkoeppe/upgrade_normaliz_pynormaliz22984
comment:54 Changed 2 years ago by
 Commit changed from 7f47ec9f71498f744614e28ed8316b72b1904bdd to 117e428c112921f5a1d555daf3a7b0ed10828532
 Status changed from needs_work to needs_review
New commits:
117e428  Upgrade normaliz to 3.5.3

comment:55 Changed 2 years ago by
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 2 years ago by
 Status changed from needs_review to positive_review
comment:57 in reply to: ↑ 46 ; followup: ↓ 58 Changed 2 years ago by
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 2 years ago by
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 2 years ago by
Ah, thanks. I didn't look at the diffs.
comment:60 Changed 2 years ago by
 Branch changed from u/mkoeppe/upgrade_normaliz_pynormaliz22984 to u/jipilab/upgrade_normaliz_pynormaliz22984
 Commit changed from 117e428c112921f5a1d555daf3a7b0ed10828532 to 7057ef457e5e6f78197100f938b8dae8d8b0c5a4
comment:61 Changed 2 years ago by
 Status changed from positive_review to needs_review
comment:62 Changed 2 years ago by
 Status changed from needs_review to positive_review
comment:63 Changed 2 years ago by
 Keywords IMAPolyGeom added
comment:64 Changed 2 years ago by
 Description modified (diff)
comment:65 Changed 2 years ago by
 Milestone changed from sage8.2 to sage8.3
comment:66 Changed 2 years ago by
 Branch changed from u/jipilab/upgrade_normaliz_pynormaliz22984 to 7057ef457e5e6f78197100f938b8dae8d8b0c5a4
 Resolution set to fixed
 Status changed from positive_review to closed
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:
Upgrade normaliz, pynormaliz
normaliz: Back out 22684_cone_reduce_memory_usage.patch