Opened 3 years ago
Closed 2 years ago
#25097 closed enhancement (fixed)
Algebraic polyhedra with Normaliz / eantic
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  geometry  Keywords:  IMAPolyGeom 
Cc:  Winfried, vdelecroix, jipilab, ghsebasguts, tmonteil, moritz  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Vincent Delecroix, JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  f94bf37 (Commits, GitHub, GitLab)  Commit:  f94bf376ea7c391fd779587968738af23c486b07 
Dependencies:  #25091, #27731, #27965  Stopgaps: 
Description (last modified by )
Implements polyhedra over embedded algebraic number fields.
 Preliminary setup within sage: install eantic, upgrade normaliz/pynormaliz (see #27682)
 the input is allowed to come from various fields, which will be coerced to a number field first via #20181:
sage: x = polygen(ZZ); P = Polyhedron(vertices=[[sqrt(2)], [AA.polynomial_root(x^32, RIF(0,3))]], backend='normaliz')
 See doctests in
src/sage/geometry/polyhedron/backend_normaliz.py
andsrc/sage/geometry/polyhedron/library.py
 For debugging help, we can write out Normaliz input files and PyNormaliz? function calls as a side effect by passing
verbose=True
.
Attachments (1)
Change History (142)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
 Cc tmonteil added
comment:3 Changed 3 years ago by
 Cc moritz added
comment:4 Changed 3 years ago by
 Branch set to u/mkoeppe/qnormalizalgebraic
comment:5 Changed 3 years ago by
 Branch changed from u/mkoeppe/qnormalizalgebraic to public/25097/qnormalizalgebraic
comment:6 Changed 3 years ago by
 Commit set to ea7a6b83b235f76a0c98930d9d1aa42991ed6d7d
 Dependencies set to #25090
 Description modified (diff)
Last 10 new commits:
117e428  Upgrade normaliz to 3.5.3

7057ef4  Merge branch to get docstring adaptation

b6ae686  Merge branch 'develop' of git://trac.sagemath.org/sage into t/25090/pynormaliz2

eaedcc2  Update PyNormaliz to 1.14

4dbdc7d  Updated the Cone calls format

1dcef4c  _init_from_normaliz_data: New, use it in _init_from_*representation

fcd8101  Updated pynormaliz to 1.15

19d8539  Polyhedron_normaliz: In verbose mode, write out normaliz format files

4cfcd14  _normaliz_format: Fix last change

ea7a6b8  Polyhedron_normaliz: Prepare classes for work over field extensions

comment:7 Changed 3 years ago by
 Description modified (diff)
comment:8 followup: ↓ 14 Changed 3 years ago by
Could someone who has worked on / has opinions on the Polyhedron constructor code help here?
What should we do for polyhedra over AA such as the following:
sage: P = polytopes.regular_polygon(3) sage: P.base_ring() Algebraic Real Field
If backend=normaliz
is requested, should we cast to the appropriate number field via #20181? Or should it be an error?
comment:9 Changed 3 years ago by
 Description modified (diff)
comment:10 Changed 3 years ago by
 Description modified (diff)
comment:11 Changed 3 years ago by
 Description modified (diff)
comment:12 Changed 3 years ago by
 Description modified (diff)
comment:13 Changed 3 years ago by
 Dependencies changed from #25090 to #25090, #20181
comment:14 in reply to: ↑ 8 Changed 3 years ago by
What should we do for polyhedra over AA such as the following:
sage: P = polytopes.regular_polygon(3) sage: P.base_ring() Algebraic Real FieldIf
backend=normaliz
is requested, should we cast to the appropriate number field via #20181? Or should it be an error?
I would say the cleanest thing to do once #20181 is done will be to use it! This probably will also simplify the constructor for field
.
comment:15 Changed 3 years ago by
 Commit changed from ea7a6b83b235f76a0c98930d9d1aa42991ed6d7d to 13d27d4d037d6009c4f0f503d42a71a76aeacb78
comment:16 Changed 3 years ago by
 Commit changed from 13d27d4d037d6009c4f0f503d42a71a76aeacb78 to 5af97b3d83e504ee9b5d50d9e7e14b737c0cf73f
Branch pushed to git repo; I updated commit sha1. New commits:
5af97b3  Polyhedron_normaliz: Separate constructors for QQ, general fields (no cnaceling)

comment:17 Changed 3 years ago by
 Description modified (diff)
comment:18 Changed 3 years ago by
I put some preliminary code in to do the coercion to number fields inside the Polyhedron_normaliz
implementation, using number_field_elements_from_algebraics
.
comment:19 Changed 3 years ago by
 Commit changed from 5af97b3d83e504ee9b5d50d9e7e14b737c0cf73f to d2aee02648bb870af22bd24f910f786275bd807b
Branch pushed to git repo; I updated commit sha1. New commits:
0f325c0  Polyhedron: Allow SR input for normaliz

aa22d44  Polyhedron_normaliz._nmz_result: New, use it instead of directly calling PyNormaliz.NmzResult

7e4f1ac  Polyhedron_normaliz: _init_from_Vrepresentation: Coerce to numberfield

0ba9f6b  Polyhedron_normaliz: Pass number field to QNormaliz

d2aee02  _init_from_normaliz_data: Separate implementations for QNormaliz, Normaliz

comment:20 Changed 3 years ago by
 Commit changed from d2aee02648bb870af22bd24f910f786275bd807b to 01d7cfc32446240c45371484acd6c12f625ca289
Branch pushed to git repo; I updated commit sha1. New commits:
2bb8958  _number_field_elements_from_algebraics_list_of_lists: New, refactor Polyhedron_normaliz using it

a9045bf  Added tests and made them pass

09df646  Merge remotetracking branch 'trac/public/algnum_to_numberfield' into t/25097/public/25097/qnormalizalgebraic

01d7cfc  lists of lists of lists of algebraic numbers, actually

comment:21 Changed 3 years ago by
 Commit changed from 01d7cfc32446240c45371484acd6c12f625ca289 to d7ecb03da0437d90e76f77642aef94376a4aa619
Branch pushed to git repo; I updated commit sha1. New commits:
d7ecb03  Temporary build scripts for normaliz, pynormaliz, pyqnormaliz

comment:22 Changed 3 years ago by
 Description modified (diff)
comment:23 Changed 3 years ago by
 Keywords IMAPolyGeom added
comment:24 Changed 3 years ago by
 Description modified (diff)
comment:25 Changed 3 years ago by
 Commit changed from d7ecb03da0437d90e76f77642aef94376a4aa619 to 79bda61224ade0f7003b17e6539c2c2f4ce451f8
comment:26 followup: ↓ 27 Changed 3 years ago by
A note: I am trying out the method in the ticket description and I get a ImportError: No module named PyQNormaliz_cpp
when I try with an example.
What should I do?
comment:27 in reply to: ↑ 26 ; followup: ↓ 28 Changed 3 years ago by
Replying to jipilab:
A note: I am trying out the method in the ticket description and I get a
ImportError: No module named PyQNormaliz_cpp
when I try with an example.What should I do?
Coming from the installation of normaliz through sage:
[normaliz] Installing ARB... [normaliz] curl: (1) Protocol "https" not supported or disabled in libcurl Makefile:2080: recipe for target 'normaliz' failed make[1]: *** [normaliz] Error 1 make[1]: Leaving directory '/home/jplabbe/sage/build/make'
comment:28 in reply to: ↑ 27 ; followup: ↓ 34 Changed 3 years ago by
Replying to jipilab:
Replying to jipilab:
A note: I am trying out the method in the ticket description and I get a
ImportError: No module named PyQNormaliz_cpp
when I try with an example.What should I do?
Coming from the installation of normaliz through sage:
[normaliz] Installing ARB... [normaliz] curl: (1) Protocol "https" not supported or disabled in libcurl Makefile:2080: recipe for target 'normaliz' failed make[1]: *** [normaliz] Error 1 make[1]: Leaving directory '/home/jplabbe/sage/build/make'
To fix this, I had to force install curl
inside a sage shell. On a debiansid distribution, it seems that it was not necessary.
comment:29 Changed 3 years ago by
 Description modified (diff)
comment:30 Changed 3 years ago by
 Description modified (diff)
comment:31 followup: ↓ 35 Changed 3 years ago by
After installing autotools
and automake
I get the following error message:
[normaliz] Installing EANTIC... [normaliz] From https://github.com/mkoeppe/eantic [normaliz] * branch winfried > FETCH_HEAD [normaliz] HEAD is now at 9de1e6f... renf_elem_class.get_den: New [normaliz] aclocal: warning: couldn't open directory 'm4': No such file or directory [normaliz] configure.ac:9: error: possibly undefined macro: AC_PROG_LIBTOOL [normaliz] If this token and others are legitimate, please use m4_pattern_allow. [normaliz] See the Autoconf documentation. [normaliz] autoreconf: /usr/bin/autoconf failed with exit status: 1 Makefile:2080: recipe for target 'normaliz' failed make[1]: *** [normaliz] Error 1 make[1]: Leaving directory '/home/jplabbe/sage/build/make'
Then, after installing libtool
(sage should complain about that not installed?!) then I get:
[normaliz] make[2]: Leaving directory '/home/jplabbe/sage/local/enfnormaliz/EANTIC_source/eantic' [normaliz] configure: error: cannot find installsh, install.sh, or shtool in "." "./.." "./../.." Makefile:2080: recipe for target 'normaliz' failed make[1]: *** [normaliz] Error 1 make[1]: Leaving directory '/home/jplabbe/sage/build/make'
comment:32 Changed 3 years ago by
... Problem solved by ghsebasguts: there was an old config file lying around. Deleting it solved the issue.
comment:33 Changed 3 years ago by
works flawlessly for me!
comment:34 in reply to: ↑ 28 Changed 3 years ago by
Replying to jipilab:
Replying to jipilab:
Replying to jipilab:
A note: I am trying out the method in the ticket description and I get a
ImportError: No module named PyQNormaliz_cpp
when I try with an example.What should I do?
Coming from the installation of normaliz through sage:
[normaliz] Installing ARB... [normaliz] curl: (1) Protocol "https" not supported or disabled in libcurl Makefile:2080: recipe for target 'normaliz' failed make[1]: *** [normaliz] Error 1 make[1]: Leaving directory '/home/jplabbe/sage/build/make'To fix this, I had to force install
curl
inside a sage shell. On a debiansid distribution, it seems that it was not necessary.
Same issue in Ubuntu 16.04LTS, doing sage f curl
in a sage shell does not work. Purging curl and reinstalling also didn't work. In order to enable ssl for curl, used the command sudo aptget install libssldev
. After that I no longer have the error curl: (1) Protocol "https" not supported...
comment:35 in reply to: ↑ 31 Changed 3 years ago by
Replying to jipilab:
After installing
autotools
andautomake
I get the following error message:[normaliz] Installing EANTIC... [normaliz] From https://github.com/mkoeppe/eantic [normaliz] * branch winfried > FETCH_HEAD [normaliz] HEAD is now at 9de1e6f... renf_elem_class.get_den: New [normaliz] aclocal: warning: couldn't open directory 'm4': No such file or directory [normaliz] configure.ac:9: error: possibly undefined macro: AC_PROG_LIBTOOL [normaliz] If this token and others are legitimate, please use m4_pattern_allow. [normaliz] See the Autoconf documentation. [normaliz] autoreconf: /usr/bin/autoconf failed with exit status: 1 Makefile:2080: recipe for target 'normaliz' failed make[1]: *** [normaliz] Error 1 make[1]: Leaving directory '/home/jplabbe/sage/build/make'Then, after installing
libtool
(sage should complain about that not installed?!) then I get:[normaliz] make[2]: Leaving directory '/home/jplabbe/sage/local/enfnormaliz/EANTIC_source/eantic' [normaliz] configure: error: cannot find installsh, install.sh, or shtool in "." "./.." "./../.." Makefile:2080: recipe for target 'normaliz' failed make[1]: *** [normaliz] Error 1 make[1]: Leaving directory '/home/jplabbe/sage/build/make'
Installed autotoolsdev
and automake
and libtool
and installation completed on Ubuntu 16.04LTS.
comment:36 Changed 3 years ago by
There is a link error in my installation:
sage: import PyQNormaliz_cpp  ImportError Traceback (most recent call last) <ipythoninput104c34a88d49d7> in <module>() > 1 import PyQNormaliz_cpp ImportError: /home/jplabbe/sage/local/enfnormaliz/lib/libantic.so.0: undefined symbol: _fmpq_poly_resultant_div
This can be solved by typing
export LD_LIBRARY_PATH=your_sage_path/local/enfnormaliz/lib/:$LD_LIBRARY_PATH
before starting sage in the terminal...
comment:37 followups: ↓ 38 ↓ 59 Changed 3 years ago by
While this solution works for jipilab, it does not for ghTimmyChan?. Apparently his Ubuntu 16.04 honors the rpath
setting from the PyQNormaliz install script.
The problem is that the rpath
will be set by python itself, and it is not possible to insert anything before that path.
comment:38 in reply to: ↑ 37 Changed 3 years ago by
Replying to ghsebasguts:
While this solution works for jipilab, it does not for ghTimmyChan?. Apparently his Ubuntu 16.04 honors the
rpath
setting from the PyQNormaliz install script.The problem is that the
rpath
will be set by python itself, and it is not possible to insert anything before that path.
Temporarily, import PyQNormaliz_cpp
can work by removing 'libflint.so.13 from
~/sage/local/lib/` to some other directory.
comment:39 Changed 3 years ago by
 Description modified (diff)
comment:40 Changed 3 years ago by
Since version 3.6.0 QNormaliz and PyQNormaliz are included in the Normaliz distribution.
comment:41 Changed 3 years ago by
 Commit changed from 79bda61224ade0f7003b17e6539c2c2f4ce451f8 to de64a4ad06db3f26bce3b535e7d0022023097b77
Branch pushed to git repo; I updated commit sha1. New commits:
855a2c0  build/pkgs/normaliz/spkginstall: Only warn if git fetch/pull fails (airplane mode)

5829bc0  src/sage/geometry/polyhedron/backend_normaliz.py: Add #optional  pyqnormaliz

de64a4a  build/pkgs/normaliz/dependencies: Depend on gc (dependency of our flint copy)

comment:42 Changed 3 years ago by
 Description modified (diff)
 Milestone changed from sage8.2 to sage8.4
comment:43 Changed 3 years ago by
 Commit changed from de64a4ad06db3f26bce3b535e7d0022023097b77 to 855391c03c327144d78c8a3dd7258e112c182c59
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
976b93f  Update normaliz to 3.6.3, pynormaliz to 1.19

e36973a  build/pkgs/normaliz/spkginstall: Update comment

98d5bd3  Merge remotetracking branch 'trac/develop' into t/25090/public/pynormaliz1.17_normaliz3.6.1

aa71e25  Merge remotetracking branch 'trac/develop' into t/25090/public/pynormaliz1.17_normaliz3.6.1

7907f7a  Polyhedron_normaliz: Fix: INPUT: items generally do not end with a period

1e20ef2  Polyhedron_normaliz._make_normaliz_cone: Add doctest

f3d11a3  Polyhedron_normaliz._init_from_normaliz_data: Add doctest

36d5473  src/sage/geometry/polyhedron/backend_normaliz.py: Add # optional  pynormaliz

e852ffa  Merge branch 't/25090/public/pynormaliz1.17_normaliz3.6.1' into t/25097/public/25097/qnormalizalgebraic

855391c  normaliz, pynormaliz, pyqnormaliz: Update build scripts to use releases

comment:44 Changed 3 years ago by
Updated this branch to use the current released versions of Normaliz from #25090 and PyQNormaliz from pypi.
comment:45 Changed 3 years ago by
 Description modified (diff)
comment:46 Changed 3 years ago by
 Commit changed from 855391c03c327144d78c8a3dd7258e112c182c59 to e34e10b8310ed9fa511a8c0f9d592da8d3785d0b
Branch pushed to git repo; I updated commit sha1. New commits:
e34e10b  Polyhedron_normaliz._nmz_result: Use the PyQNormaliz conversion handlers

comment:47 Changed 3 years ago by
Far from "needs review", but needs testing / debugging. (I'm currently getting a segfault when running the testsuite for backend_normaliz.py.)
comment:48 Changed 3 years ago by
Could you give more specific information?
comment:49 Changed 3 years ago by
 Description modified (diff)
sage: x = polygen(ZZ); P = Polyhedron(vertices=[[sqrt(2)], [AA.polynomial_root(x^32, RIF(0,3))]], backend='normaliz', ver ....: bose=True) # 8< Equivalent QNormaliz input file 8< amb_space 1 subspace 0 vertices 2 (a^3) 1 (a^2) 1 number_field min_poly (a^6  2) embedding [1.122462048309373 +/ 5.38e16] cone 0 # 8<8<8< # Calling PyQNormaliz_cpp.NmzCone(**{'subspace': [], 'vertices': [[[[0L, 1L], [0L, 1L], [0L, 1L], [1L, 1L], [0L, 1L], [0L, 1L]], 1L], [[[0L, 1L], [0L, 1L], [1L, 1L], [0L, 1L], [0L, 1L], [0L, 1L]], 1L]], 'number_field': 'min_poly (a^6  2) embedding [1.122462048309373 +/ 5.38e16]', 'cone': []})  (no backtrace available)  Unhandled SIGSEGV: A segmentation fault 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.  Segmentation fault: 11
comment:50 followup: ↓ 51 Changed 3 years ago by
Normaliz 3.6.3 (and presumably already 3.6.0) computes the polytope without any problems, provided the number_field follows immediately after amb_space. (One could think of relaxing this condition.) Output:
Real embedded number field: min_poly (a^6  2) embedding [1.1225 +/ 4.57e5] 2 vertices of polyhedron 0 extreme rays of recession cone 2 support hyperplanes of polyhedron (homogenized) embedding dimension = 2 affine dimension of the polyhedron = 1 (maximal) rank of recession cone = 0 dehomogenization: 0 1 *********************************************************************** 2 vertices of polyhedron: (a^2 ~ 1.2599) 1 (a^3 ~ 1.4142) 1 0 extreme rays of recession cone: 2 support hyperplanes of polyhedron (homogenized): (1/2*a^3 ~ 0.7071) 1 (1/2*a^4 ~ 0.7937) 1
comment:51 in reply to: ↑ 50 Changed 3 years ago by
Replying to Winfried:
Normaliz 3.6.3 (and presumably already 3.6.0) computes the polytope without any problems,
Thanks for testing. Yes, I think the segfault is coming from somewhere in the interfacing.
provided the number_field follows immediately after amb_space. (One could think of relaxing this condition.)
No, I don't think that's necessary  I just need to fix the normaliz file writer in my sage code to know about this.
comment:52 Changed 3 years ago by
 Commit changed from e34e10b8310ed9fa511a8c0f9d592da8d3785d0b to b6a16657020f71a1ffe0f4011c02a263f1128f87
Branch pushed to git repo; I updated commit sha1. New commits:
b6a1665  Polyhedron_normaliz._nmz_result: Fix up NumberfieldElementHandler

comment:53 Changed 3 years ago by
x = polygen(ZZ); P = Polyhedron(vertices=[[sqrt(2)], [AA.polynomial_root(x^32, RIF(0,3))]], backend='normaliz', verbose=True); P._Hrepresentation .... segfault ...
Some memory corruption involving the gc, it seems
comment:54 Changed 3 years ago by
 Commit changed from b6a16657020f71a1ffe0f4011c02a263f1128f87 to ec5f5c9249da26235725d166d897514787bf9992
Branch pushed to git repo; I updated commit sha1. New commits:
ec5f5c9  Polyhedron_normaliz._normaliz_format: Update code that puts 'number_field' right after 'amb_space'

comment:55 Changed 3 years ago by
 Commit changed from ec5f5c9249da26235725d166d897514787bf9992 to 9669a6f17b687480bd94584b4cb5d437d4c34767
Branch pushed to git repo; I updated commit sha1. New commits:
9669a6f  Polyhedron_normaliz: If base_ring is already a number field, use it

comment:56 Changed 3 years ago by
 Commit changed from 9669a6f17b687480bd94584b4cb5d437d4c34767 to 3b454e23e9d0b8259135d3fbe5607985e9ff0fc4
comment:57 Changed 3 years ago by
 Dependencies changed from #25090, #20181 to #25090, #20181, #25171
 Description modified (diff)
comment:58 Changed 3 years ago by
 Commit changed from 3b454e23e9d0b8259135d3fbe5607985e9ff0fc4 to eb6646e889bb9a706192726000cc7a0ad0c78811
Branch pushed to git repo; I updated commit sha1. New commits:
46dfa08  Add missing build/pkgs/pyqnormaliz/checksums.ini file

8b4decc  Experimental upgrade of FLINT to 2.5.2+trunk20180918e12c83de3e100a113dbc030894d89f67dfe83cd4; remove patches

2e3fe7d  Merge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormalizalgebraic

41f6f98  build/{normaliz,pyqnormaliz}/spkginstall: Use FLINT from #25171, use sage arb, get rid of enfnormaliz/ install hierarchy

13089b4  build/pkgs/flint/checksums.ini: Update for using github release tarball

eb6646e  Merge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormalizalgebraic

comment:59 in reply to: ↑ 37 Changed 3 years ago by
Replying to ghsebasguts:
While this solution works for jipilab, it does not for ghTimmyChan?. Apparently his Ubuntu 16.04 honors the
rpath
setting from the PyQNormaliz install script.The problem is that the
rpath
will be set by python itself, and it is not possible to insert anything before that path.
With the latest commits, I have removed the separate flint installation and this ticket should be easier to build. Please test
comment:60 Changed 3 years ago by
 Commit changed from eb6646e889bb9a706192726000cc7a0ad0c78811 to 95b5bdd0a3ca6a6fc2cbedfed2ef4f2062b7bcf0
Branch pushed to git repo; I updated commit sha1. New commits:
1ccf632  polytopes.snub_cube: Add keyword arg verbose

25e64fc  polytopes.snub_dodecahedron: Add keyword arg verbose, mark pyqnormaliz doctest 'not tested  very long time'

7dfd8a5  polytopes.six_hundred_cell: Fix wrong doctest

95b5bdd  polytopes.snub_cube: Add keywords exact, base_ring. Add pyqnormaliz doctest

comment:61 Changed 3 years ago by
 Commit changed from 95b5bdd0a3ca6a6fc2cbedfed2ef4f2062b7bcf0 to 3d8d078f7592dd4063d56d6459d64cfe015f8bbc
Branch pushed to git repo; I updated commit sha1. New commits:
1b977dc  Polyhedron_normaliz: Fix a doctest

4eaec75  Polyhedron_normaliz: Sort keys in verbose output for predictable doctests

7749ace  Polyhedron_normaliz: Guard against deadly exceptions in *_handler

a692e85  Polyhedron_QQ_normaliz._init_from_normaliz_data: Add doctests

3040b2d  Polyhedron_normaliz: Refactor using format_number_field_data, passing normaliz_field through _init_from_normaliz_data

d56e0d4  Polyhedron_normaliz: Refactor, passing normaliz_field through _init_from_normaliz_cone.

85166c3  Polyhedron_normaliz: New init arg, normaliz_field. Fix _init_from_normaliz_data doctest

bb0f73d  Polyhedron_normaliz: Extend doctest

3d8d078  Polyhedron_normaliz: Switch between Normaliz, QNormaliz based on normaliz_field, rather than class

comment:62 Changed 3 years ago by
 Description modified (diff)
comment:63 Changed 3 years ago by
 Commit changed from 3d8d078f7592dd4063d56d6459d64cfe015f8bbc to ad26e475628de592be3d6f878c187c9e34b5dc8a
Branch pushed to git repo; I updated commit sha1. New commits:
ad26e47  PyQNormaliz: Update to 1.2

comment:64 Changed 3 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:65 Changed 3 years ago by
comment:66 Changed 3 years ago by
 Commit changed from ad26e475628de592be3d6f878c187c9e34b5dc8a to 725ea5baae0cc2922baa0480e4c8cd7d88c9239d
Branch pushed to git repo; I updated commit sha1. New commits:
725ea5b  Merge branch 'trac/develop' into t/25097/public/25097/qnormalizalgebraic

comment:67 Changed 3 years ago by
 Commit changed from 725ea5baae0cc2922baa0480e4c8cd7d88c9239d to bd3c990559c1531ecf14d98ba7b009d26504a05e
comment:68 followup: ↓ 71 Changed 2 years ago by
I think we should decouple: upgrading Normaliz and interfacing Normaliz.
comment:69 Changed 2 years ago by
 Dependencies changed from #25090, #20181, #25171 to #25090, #20181, #25171, #27682
I opened #27682
comment:70 Changed 2 years ago by
 Dependencies changed from #25090, #20181, #25171, #27682 to #20181, #27682
 Description modified (diff)
 Milestone changed from sage8.4 to sage8.8
comment:71 in reply to: ↑ 68 Changed 2 years ago by
comment:72 Changed 2 years ago by
I think it would be good to split this ticket into two tickets:
 Make
backend='normaliz'
accepts input being in a common embedded real number field. That is the following should worksage: K.<a> = QuadraticField(2, embedding=AA(2)**(1/2)) sage: Polyhedron([[0,0],[1,a],[0,2*a],[1,a]], base_ring=K, backend='normaliz')
And we should also make it so thatbackend='normaliz'
is actually automatically chosen if availablesage: Polyhedron([[0,0],[1,a],[0,2*a],[1,a]], base_ring=K) # should use normaliz
This would be enough to generate any of the regular polytope examples exactly (including the 600 cells) that could go in the SageMath databasepolytopes.
.  (followup ticket) When the input are elements in
AA
, or symbolic algebraic, etc, try to figure out how to convert stuff into an embedded number field
comment:73 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:74 Changed 2 years ago by
 Commit changed from bd3c990559c1531ecf14d98ba7b009d26504a05e to 2f3c42c418acfd67250f3db2b517cb125dc45a5b
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
61e4df9  Fix docstring even more

9255f2c  pyflakes

9a06e8e  Merge branch 'public/algnum_to_numberfield' of git://trac.sagemath.org/sage into t/20181/public/algnum_to_numberfield

f8a2d52  Merge branch 't/20181/public/algnum_to_numberfield' into t/25097/public/25097/qnormalizalgebraic

46fcf67  Merge tag '8.8.beta3' into t/25171/experimental_flint_upgrade_package_tracking_trunk

b320692  Merge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormalizalgebraic

76d7db1  Add patch

c3fd47f  Merge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormalizalgebraic

0c93e30  Add FLINT patch install_name_tool_destdir.patch

2f3c42c  Merge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormalizalgebraic

comment:75 Changed 2 years ago by
I have updated this ticket for 8.8.beta3. Still using the old Normaliz/QNormaliz/PyNormaliz/PyQNormaliz etc., but merged in #20181. 3 doctest failures.
comment:76 followup: ↓ 77 Changed 2 years ago by
 Commit changed from 2f3c42c418acfd67250f3db2b517cb125dc45a5b to 7c3d64c8a12275511f3d3875d37f48d3cb6857ec
Branch pushed to git repo; I updated commit sha1. New commits:
7c3d64c  Fix up for exact snub cube construction

comment:77 in reply to: ↑ 76 Changed 2 years ago by
comment:78 Changed 2 years ago by
#27716 has all the backend work that does not depend on the algebraic stuff.
comment:79 followup: ↓ 81 Changed 2 years ago by
comment:80 Changed 2 years ago by
 Commit changed from 7c3d64c8a12275511f3d3875d37f48d3cb6857ec to ec695ac88bfb272dc05444a74579da8fdc0683ab
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
57a9d10  add exact option to snub_cube

8703f7f  fix snub cube example in polyhedra_tutorial.rst

7618ea9  package eantic

d752708  upgrade normaliz

4eb02f8  fix doctest in backend_normaliz.py

56a903e  Merge branches 't/27716/public/normalizbackendrefactoring', 't/26340/public/26340', 't/27682/27682' and 't/20181/public/algnum_to_numberfield' into t/25097/public/normaliz3.7.0algebraic

915cb46  backend_normaliz: algebraic polyhedra

3a6e39d  Add library polytope tests for algebraic polytopes

c7aa2ef  change use of PyQNormaliz to PyNormaliz

ec695ac  Use _number_field_triple instead of _format_number_field_data

comment:81 in reply to: ↑ 79 Changed 2 years ago by
comment:82 Changed 2 years ago by
 Commit changed from ec695ac88bfb272dc05444a74579da8fdc0683ab to a7e3af33495b528418de2af88d8b5b0eedc9d7dd
comment:83 Changed 2 years ago by
All tests work now except for one that needs a fix for this: https://github.com/Normaliz/PyNormaliz/issues/41
comment:84 Changed 2 years ago by
 Dependencies changed from #20181, #27682 to #27716, #26340, #27682, #20181
comment:85 Changed 2 years ago by
With Normaliz master (unreleased), all tests pass except for the broken snub cube example from #26340.
comment:86 Changed 2 years ago by
 Commit changed from a7e3af33495b528418de2af88d8b5b0eedc9d7dd to 19bb5d3ca9b2fcbec6f040ebc1f527689f104d74
Branch pushed to git repo; I updated commit sha1. New commits:
3ce403d  Change optional  pyqnormaliz to pynormaliz

b370268  pyflakes fixes

328a04a  _nmz_result: Add doctest

ee5ff41  Mark newest doctest # optional  pynormaliz

b60f653  Merge branch 't/27716/public/normalizbackendrefactoring' into t/25097/public/normaliz3.7.0algebraic

05867b4  Merge tag '8.8.beta3' into t/26340/public/26340

39c3c67  Fix construction of snub_cube data

f52802f  Merge branch 't/26340/public/26340' into t/25097/public/normaliz3.7.0algebraic

19bb5d3  Add snub_cube test with normaliz

comment:87 Changed 2 years ago by
 Commit changed from 19bb5d3ca9b2fcbec6f040ebc1f527689f104d74 to 706e79648c58b2f9f69b5b9cc667a0c732096795
comment:88 Changed 2 years ago by
 Commit changed from 706e79648c58b2f9f69b5b9cc667a0c732096795 to e3bbecda2f444ef6c21938a8bef3e4bd80095cb2
comment:89 Changed 2 years ago by
 Commit changed from e3bbecda2f444ef6c21938a8bef3e4bd80095cb2 to 3ad690512e703b8868c36fa890cb5a477c5e0659
comment:90 Changed 2 years ago by
 Commit changed from 3ad690512e703b8868c36fa890cb5a477c5e0659 to e3bbecda2f444ef6c21938a8bef3e4bd80095cb2
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
comment:91 Changed 2 years ago by
 Commit changed from e3bbecda2f444ef6c21938a8bef3e4bd80095cb2 to 24a87469967c0d47967c5214e2256cae7147b00b
Branch pushed to git repo; I updated commit sha1. New commits:
eaa4584  package eantic 0.1.3b0

e5c669d  upgrade normaliz

14d6c83  Update normaliz to 3.7.1, pynormaliz to 2.1

64f1f19  fix doctest in backend_normaliz.py

59e069b  Merge branch 't/27682/27682' into t/25097/public/normaliz3.7.0algebraic

24a8746  Update error message in doctest for new PyNormaliz

comment:92 Changed 2 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
 Summary changed from Algebraic polyhedra with QNormaliz / eantic to Algebraic polyhedra with Normaliz / eantic
Ready for review. Builds and tests OK on macOS Mojave.
comment:93 Changed 2 years ago by
Builds on debian stretch.
$ sage t sage/src/sage/geometry/polyhedron/*.py optional=bliss,dochtml,gfortran,lrslib,memlimit,mpir,ninja_build,normaliz,pynormaliz,python2,sage,topcom too few successful tests, not using stored timings Git branch: 25097 Using optional=bliss,dochtml,gfortran,lrslib,memlimit,mpir,ninja_build,normaliz,pynormaliz,python2,sage,topcom Doctesting 25 files.  All tests passed! 
comment:94 Changed 2 years ago by
 Commit changed from 24a87469967c0d47967c5214e2256cae7147b00b to f232c6cec018358b144f0f8b9541ae7b2fab8699
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
160dd88  Merge branch 't/27682/27682' into t/25091/public/some_normaliz_features

78c2de4  Merge branch 't/27716/public/normalizbackendrefactoring' into t/25091/public/some_normaliz_features

41bd968  Fix doctest error message  NormalizError

7662077  Fixup merge

885ef9e  Use EhrhartQuasiPolynomial

313fee1  Fix doctest of hilbert_series

7905903  Pass grading as a matrix

9249dda  Use _nmz_result instead of calling NmzResult directly

ec96615  Merge branch 't/25091/public/some_normaliz_features' into t/25097/public/normaliz3.7.0algebraic

f232c6c  _nmz_result already constructs rationals

comment:95 Changed 2 years ago by
 Dependencies changed from #27716, #26340, #27682, #20181 to #27716, #26340, #27682, #20181, #25091
Merged #25091, works fine after minor changes.
comment:96 Changed 2 years ago by
 Commit changed from f232c6cec018358b144f0f8b9541ae7b2fab8699 to 812395cd57129ae1832562b23950ead1a1e6d97f
comment:97 Changed 2 years ago by
 Commit changed from 812395cd57129ae1832562b23950ead1a1e6d97f to 1dd5b3c3ce8cc2e23702188c035eadd2cbec406c
comment:98 Changed 2 years ago by
 Dependencies changed from #27716, #26340, #27682, #20181, #25091 to #27716, #26340, #27682, #20181, #25091, #27731
comment:99 Changed 2 years ago by
 Commit changed from 1dd5b3c3ce8cc2e23702188c035eadd2cbec406c to ac4551155be9dec669518499c2fd6c6d563c9a31
Branch pushed to git repo; I updated commit sha1. New commits:
ac45511  Fix for tuples in H and V representations

comment:100 Changed 2 years ago by
That error was caused by using an old PyNormaliz?.
comment:101 Changed 2 years ago by
 Commit changed from ac4551155be9dec669518499c2fd6c6d563c9a31 to 2c3ea3c33f79e5fce5875efc8e6f1c474f907802
comment:102 Changed 2 years ago by
 Commit changed from 2c3ea3c33f79e5fce5875efc8e6f1c474f907802 to 59f320b2fb7aa1ff552bf0d76dec5a4d48d1c5d2
comment:103 Changed 2 years ago by
 Commit changed from 59f320b2fb7aa1ff552bf0d76dec5a4d48d1c5d2 to ec2646dc2f2389af7bf09a364152920ecba7b9be
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
59cc7e0  Fix Error messages

a4f01d4  Fixed typo and added require package

fbd3dce  removed is_package_installed

8ceee84  Change TypeError for RuntimeError

0da067f  Fixed doctests and added author name

d7122d7  pyflakes

63a80b6  Forgot an import

d8716e3  Refactor NmzResult

08b315b  Fixed infinite recursion

ec2646d  Merge branch 't/25091/public/some_normaliz_features' into t/25097/public/normaliz3.7.0algebraic

comment:104 followup: ↓ 115 Changed 2 years ago by
Here is an error happening while coercing the values back into the given base ring (UCF
in this case):
sage: W = CoxeterGroup(['I',8]) sage: n = W.one().canonical_matrix().rank() sage: weights = W.fundamental_weights() sage: point = [ZZ.one()] * n sage: apex = sum(point[i1] * weights[i] for i in weights.keys()) sage: non_zero_index = list(apex).index(filter(lambda x: x!=0, apex)[0]) sage: apex = (QQ(1)/apex[non_zero_index]) * apex sage: apex.set_immutable() sage: vertices = set() sage: br = apex.base_ring() sage: for w in W: ....: new_point = w * apex ....: new_point.set_immutable() ....: vertices.add(new_point) ....: sage: Polyhedron(vertices=vertices, backend='normaliz', base_ring=br,verbose=True) Traceback (most recent call last): ... TypeError: 1 of type <type 'sage.rings.number_field.number_field_element.NumberFieldElement_absolute'> not valid to initialize an element of the universal cyclotomic field
comment:105 Changed 2 years ago by
Unfortunately both of the following conversions fail:
UniversalCyclotomicField()(AA(1)) TypeError: 1 of type <class 'sage.rings.qqbar.AlgebraicReal'> not valid to initialize an element of the universal cyclotomic field
and
sage: K.<sqrt2> = QuadraticField(2) sage: UniversalCyclotomicField()(K(1)) TypeError: 1 of type <type 'sage.rings.number_field.number_field_element_quadratic.NumberFieldElement_quadratic'> not valid to initialize an element of the universal cyclotomic field
comment:106 Changed 2 years ago by
This should be a separate ticket
comment:107 Changed 2 years ago by
 Commit changed from ec2646dc2f2389af7bf09a364152920ecba7b9be to 978765bd816698be8d73ed04db26a2206ea8d38f
Branch pushed to git repo; I updated commit sha1. New commits:
292e01f  backend_normaliz: py3 fix

d44a00d  backend_normaliz: Canonicalize sort order of fields in normaliz files

dd9a81b  Merge tag '8.8.beta4' into t/27731/public/27731

9b3a5ba  Fix error message in doctest for new pynormaliz version

0a8c6f8  Merge branch 't/27782/1_pynormaliz_doctest_failing_in_src_sage_geometry_polyhedron_backend_normaliz_py' into t/27731/public/27731

978765b  Merge branch 't/27731/public/27731' into t/25097/public/normaliz3.7.0algebraic

comment:108 Changed 2 years ago by
 Status changed from needs_review to needs_work
There are python3 issues, pyflakes conventions issues, :::
issues, and 2 missing doctests in backend_normaliz.py
, see patchbot for more details.
From a recent bot, there are the following files that have failing doctests (seems like the optional hashtag is missing):
sage t long src/sage/geometry/polyhedron/backend_normaliz.py # 11 doctests failed sage t long src/doc/en/thematic_tutorials/geometry/polyhedra_tutorial.rst # 2 doctests failed
comment:109 Changed 2 years ago by
 Commit changed from 978765bd816698be8d73ed04db26a2206ea8d38f to a8f307e454cd16ef20850b99057f22c368629e4e
comment:110 Changed 2 years ago by
Merged in latest version of #25091 and current beta.
comment:111 Changed 2 years ago by
 Dependencies changed from #27716, #26340, #27682, #20181, #25091, #27731 to #25091
comment:112 Changed 2 years ago by
 Commit changed from a8f307e454cd16ef20850b99057f22c368629e4e to ed6fcbf1ab93216403c74448b275fcdf74b12ae5
comment:113 Changed 2 years ago by
 Status changed from needs_work to needs_review
Setting it back to needs_review to get a fresh patchbot run.
comment:114 Changed 2 years ago by
 Commit changed from ed6fcbf1ab93216403c74448b275fcdf74b12ae5 to 4bd7635c5506c2de2df92f5674fab61adf5f698c
Branch pushed to git repo; I updated commit sha1. New commits:
4bd7635  Add more optional  pynormaliz

comment:115 in reply to: ↑ 104 ; followup: ↓ 122 Changed 2 years ago by
Replying to jipilab:
Here is an error happening while coercing the values back into the given base ring (
UCF
in this case):
Should we actually allow a complex field such as UCF as the base ring of a polyhedron?
comment:116 Changed 2 years ago by
 Commit changed from 4bd7635c5506c2de2df92f5674fab61adf5f698c to b484af77e078358d50be4ed0a4eba92df7872c8d
comment:117 Changed 2 years ago by
 Commit changed from b484af77e078358d50be4ed0a4eba92df7872c8d to 99e12c15bf28965b61487dd99b8c720aa00c09bc
Branch pushed to git repo; I updated commit sha1. New commits:
99e12c1  Polyhedron_normaliz._number_field_triple: Add doctest

comment:118 Changed 2 years ago by
 Commit changed from 99e12c15bf28965b61487dd99b8c720aa00c09bc to 4d82f5f5fd075aeea0be7965853fd3b557481166
Branch pushed to git repo; I updated commit sha1. New commits:
4d82f5f  _number_field_elements_from_algebraics_list_of_lists_of_lists: Remove debugging code

comment:119 Changed 2 years ago by
 Commit changed from 4d82f5f5fd075aeea0be7965853fd3b557481166 to fcf2c353f0e99ca2e72a6da98d3b6e43689229fc
comment:120 Changed 2 years ago by
 Dependencies changed from #25091 to #25091, #27731
comment:121 Changed 2 years ago by
 Commit changed from fcf2c353f0e99ca2e72a6da98d3b6e43689229fc to abb7b02fd0e2866b1cc5627d9046f89074ee7b09
Branch pushed to git repo; I updated commit sha1. New commits:
abb7b02  More optional  pynormaliz

comment:122 in reply to: ↑ 115 Changed 2 years ago by
Replying to mkoeppe:
Replying to jipilab:
Here is an error happening while coercing the values back into the given base ring (
UCF
in this case):Should we actually allow a complex field such as UCF as the base ring of a polyhedron?
Not sure. I remember I was not so happy about allowing real algebraic numbers that have a complex parent into polyhedron: it makes things quite intricate in designing the constructor.
The situation before this ticket was not allowing anything inside of SymbolicRing
for example, and this would also do the same for UCF
is the backend field was not given (or perhaps there was an effort to check if one can coerce it in AA
)... Now, since we accept things that are in the symbolic ring if it is possible to coerce it into an embedded number field... I feel like it would be nice to accept elements of UCF
if they are real... I remember I had to tweak #20181 so that it would manage to coerce real algebraic numbers even though they are elements of a complex ring.
That said, I would say that the answer should be 'No' (as far as I understand, complex polyhedra are not really welldefined, or are they?). So, this would be an exception to the idea "give back a common parent to the elements given by the user and use it as base ring". I guess the rule should be bound to ultimately "stay real".
So there could be a check to see if the user provided elements from a complex ring (although they are indeed real) and do the appropriate steps to give a real base ring. I do not know how hard implementing this here.
comment:123 followup: ↓ 124 Changed 2 years ago by
 Status changed from needs_review to needs_work
There are two doctest failing in the tutorials (coming from changing an error message):
 sage t long src/sage/sat/boolean_polynomials.py # 1 doctest failed sage t long src/sage/graphs/generic_graph.py # 1 doctest failed sage t long src/doc/en/thematic_tutorials/geometry/polyhedra_tutorial.rst # 2 doctests failed 
The first two might be spurious errors, but maybe they come from employing polyhedra...
comment:124 in reply to: ↑ 123 Changed 2 years ago by
Replying to jipilab:
There are two doctest failing in the tutorials (coming from changing an error message):
 sage t long src/sage/sat/boolean_polynomials.py # 1 doctest failed sage t long src/sage/graphs/generic_graph.py # 1 doctest failed sage t long src/doc/en/thematic_tutorials/geometry/polyhedra_tutorial.rst # 2 doctests failed The first two might be spurious errors, but maybe they come from employing polyhedra...
Actually, there is some dirt in the traceback for the errors on the branch. Most probably this is the cause of the breaking doctests...
comment:125 Changed 2 years ago by
 Commit changed from abb7b02fd0e2866b1cc5627d9046f89074ee7b09 to ca71cc29875892438eb8d77393b5ec1635f015b0
comment:126 Changed 2 years ago by
 Status changed from needs_work to needs_review
Fixed the doctest and merged in the newest beta.
comment:127 Changed 2 years ago by
 Status changed from needs_review to needs_work
There is 1 doctest failing because of #21161. :/
comment:128 Changed 2 years ago by
 Reviewers set to JeanPhilippe Labbé
comment:129 Changed 2 years ago by
comment:130 Changed 2 years ago by
 Dependencies changed from #25091, #27731 to #25091, #27731, #27965
#27965 should be merged into this ticket to avoid yet another conflict.
The other ticket allows to "whitelist" geometry for py3, probably asap for it to be in the next release.
... that said, I would also like to see this ticket in the next release so that we can move on to further enrich normaliz' interfacing...
comment:131 Changed 2 years ago by
 Milestone changed from sage8.8 to sage8.9
Tickets still needing working or clarification should be moved to the next release milestone at the soonest (please feel free to revert if you think the ticket is close to being resolved).
comment:132 Changed 2 years ago by
 Commit changed from ca71cc29875892438eb8d77393b5ec1635f015b0 to 4258704205e49d5786850ef657465a7e3185f0c0
Branch pushed to git repo; I updated commit sha1. New commits:
28b8dbd  Merge tag '8.8.rc1' into t/25097/public/normaliz3.7.0algebraic

6fcdf05  trac 27965: py3 fix in geometry/polyhedron/library.py

3d46bb2  Merge branch 't/27965/27965' into t/25097/public/normaliz3.7.0algebraic

4258704  Fix doctests for changed embedded numberfield repr

comment:133 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:134 Changed 2 years ago by
 Reviewers changed from JeanPhilippe Labbé to Vincent Delecroix, JeanPhilippe Labbé
Looks good to me now. I would say this is ready and I would put it as positive review, with Vincent also as a reviewer too.
@Vincent: Let us know if you agree. It would be good to know if there are last things to be aware of.
comment:135 Changed 2 years ago by
ping!
Any last comments Vincent?
comment:136 Changed 2 years ago by
 Status changed from needs_review to needs_work
doctest failure in /library with py3
comment:137 Changed 2 years ago by
 Commit changed from 4258704205e49d5786850ef657465a7e3185f0c0 to f94bf376ea7c391fd779587968738af23c486b07
comment:138 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:139 Changed 2 years ago by
The last changes looks reasonable to me.
comment:140 Changed 2 years ago by
 Status changed from needs_review to positive_review
It looks good to me too. I set it to positive review.
Just a FYI: if there are any other comments, next week is the Sage days in Bonn and vdelecroix and me are going to be available to work on followups if need be.
comment:141 Changed 2 years ago by
 Branch changed from public/25097/qnormalizalgebraic to f94bf376ea7c391fd779587968738af23c486b07
 Resolution set to fixed
 Status changed from positive_review to closed
prerequisite: package antic (that has no release yet).