Opened 4 years ago
Closed 3 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: | sage-8.3 |
Component: | packages: optional | Keywords: | IMA-PolyGeom |
Cc: | Winfried, tmonteil, vdelecroix, tscrim, jipilab, gh-sebasguts | 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: |
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/normaliz-3.5.3.tar.gz
- https://pypi.python.org/packages/30/48/d2ad6e4defcce3959c66ee541cf5c0eda12186e42ecd68a0531813a16bd0/PyNormaliz-1.12.tar.gz#md5=1ec3419eaa061a0ee4b4f1ffca9cb44d
Upstream bug: https://github.com/Normaliz/Normaliz/issues/176
Follow-up ticket with more cutting-edge PyNormaliz
version: #25090.
Change History (66)
comment:1 Changed 4 years ago by
- Description modified (diff)
- Milestone changed from sage-8.0 to sage-8.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 4 years ago by
- Branch set to u/mkoeppe/upgrade_normaliz_to_3_4_0_and_pynormaliz_to_1_7
comment:3 Changed 4 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 4 years ago by
- Description modified (diff)
comment:5 Changed 4 years ago by
comment:6 Changed 4 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 4 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 3 years ago by
- Description modified (diff)
- Milestone changed from sage-8.1 to sage-8.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 3 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 3 years ago by
- Cc vdelecroix tscrim jipilab added
- Status changed from new to needs_review
comment:11 Changed 3 years ago by
- Description modified (diff)
comment:12 Changed 3 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/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 3 years ago by
- Commit changed from f2137cc5bb49278657ce23f0f8c775400b494023 to b8f7abc04085155225e54ad168ce4a7884fdd6cf
Branch pushed to git repo; I updated commit sha1. New commits:
b8f7abc | normaliz/spkg-install: Explain --enable-flint
|
comment:14 Changed 3 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 3 years ago by
Upstream master branch (8db5b68491a105b9185a01c29c2f126f5115e238) has different behavior but does not fix this.
comment:16 Changed 3 years ago by
- Reviewers changed from Travis Scrimshaw to Travis 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 3 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 3 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 3 years ago by
Unfortunately Normaliz crashes when coordinate 1 is interpreted as the RHS. Will take care of it.
comment:20 follow-up: ↓ 21 Changed 3 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 3 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 3 years ago by
- Branch changed from u/mkoeppe/upgrade_normaliz_to_3_4_0_and_pynormaliz_to_1_7 to u/tscrim/upgrade_noramliz_pynormaliz-22984
- 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 3 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 3 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 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 3 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 3 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 3 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 3 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 follow-up: ↓ 31 Changed 3 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()<EmbDim-1)
But againthis has nothing to do with the wrong number of lattice points.
comment:30 Changed 3 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 3 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 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.
comment:32 Changed 3 years ago by
I think all the observed problems are solved. More tomorrow.
comment:33 Changed 3 years ago by
Great, thank you!
comment:34 Changed 3 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 3 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 follow-up: ↓ 37 Changed 3 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 3 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 3 years ago by
- Commit changed from 737552d95ff6d810c56611560fa9b62f888cdafc to 57d40b030deb164404f6a245af0c3734ebf1eb40
comment:39 Changed 3 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 3 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 3 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 3 years ago by
ping
comment:44 Changed 3 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 3 years ago by
- Cc gh-sebasguts added
comment:46 follow-up: ↓ 57 Changed 3 years ago by
- Reviewers changed from Travis Scrimshaw, Thierry Monteil to Travis 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 3 years ago by
- Branch changed from u/tscrim/upgrade_noramliz_pynormaliz-22984 to u/mkoeppe/upgrade_noramliz_pynormaliz-22984
comment:48 Changed 3 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 3 years ago by
- Branch changed from u/mkoeppe/upgrade_noramliz_pynormaliz-22984 to u/jipilab/upgrade_normaliz_pynormaliz-22984
- Commit changed from 183087a21eaaf64b464fa6ffe0aaf94819d6626c to 7f47ec9f71498f744614e28ed8316b72b1904bdd
- Status changed from needs_review to positive_review
New commits:
7f47ec9 | Adapted the polyhedron docstring
|
comment:50 Changed 3 years ago by
It looks good to me now and all tests pass in the polyhedron folder.
comment:51 Changed 3 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 3 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 3 years ago by
- Branch changed from u/jipilab/upgrade_normaliz_pynormaliz-22984 to u/mkoeppe/upgrade_normaliz_pynormaliz-22984
comment:54 Changed 3 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 3 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 3 years ago by
- Status changed from needs_review to positive_review
comment:57 in reply to: ↑ 46 ; follow-up: ↓ 58 Changed 3 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 3 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 3 years ago by
Ah, thanks. I didn't look at the diffs.
comment:60 Changed 3 years ago by
- Branch changed from u/mkoeppe/upgrade_normaliz_pynormaliz-22984 to u/jipilab/upgrade_normaliz_pynormaliz-22984
- Commit changed from 117e428c112921f5a1d555daf3a7b0ed10828532 to 7057ef457e5e6f78197100f938b8dae8d8b0c5a4
comment:61 Changed 3 years ago by
- Status changed from positive_review to needs_review
comment:62 Changed 3 years ago by
- Status changed from needs_review to positive_review
comment:63 Changed 3 years ago by
- Keywords IMA-PolyGeom added
comment:64 Changed 3 years ago by
- Description modified (diff)
comment:65 Changed 3 years ago by
- Milestone changed from sage-8.2 to sage-8.3
comment:66 Changed 3 years ago by
- Branch changed from u/jipilab/upgrade_normaliz_pynormaliz-22984 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