Opened 2 years ago

Closed 9 months ago

#25097 closed enhancement (fixed)

Algebraic polyhedra with Normaliz / e-antic

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-8.9
Component: geometry Keywords: IMA-PolyGeom
Cc: Winfried, vdelecroix, jipilab, gh-sebasguts, tmonteil, moritz Merged in:
Authors: Matthias Koeppe Reviewers: Vincent Delecroix, Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: f94bf37 (Commits) Commit: f94bf376ea7c391fd779587968738af23c486b07
Dependencies: #25091, #27731, #27965 Stopgaps:

Description (last modified by mkoeppe)

Implements polyhedra over embedded algebraic number fields.

  • Preliminary setup within sage: install e-antic, 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^3-2, RIF(0,3))]], backend='normaliz')
    
  • See doctests in src/sage/geometry/polyhedron/backend_normaliz.py and src/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)

error.txt (5.8 KB) - added by jipilab 11 months ago.
An error in convex hull?

Download all attachments as: .zip

Change History (142)

comment:1 Changed 2 years ago by vdelecroix

prerequisite: package antic (that has no release yet).

comment:2 Changed 2 years ago by tmonteil

  • Cc tmonteil added

comment:3 Changed 2 years ago by jipilab

  • Cc moritz added

comment:4 Changed 2 years ago by mkoeppe

  • Branch set to u/mkoeppe/qnormaliz-algebraic

comment:5 Changed 2 years ago by mkoeppe

  • Branch changed from u/mkoeppe/qnormaliz-algebraic to public/25097/qnormaliz-algebraic

comment:6 Changed 2 years ago by mkoeppe

  • Commit set to ea7a6b83b235f76a0c98930d9d1aa42991ed6d7d
  • Dependencies set to #25090
  • Description modified (diff)

Last 10 new commits:

117e428Upgrade normaliz to 3.5.3
7057ef4Merge branch to get docstring adaptation
b6ae686Merge branch 'develop' of git://trac.sagemath.org/sage into t/25090/pynormaliz2
eaedcc2Update PyNormaliz to 1.14
4dbdc7dUpdated the Cone calls format
1dcef4c_init_from_normaliz_data: New, use it in _init_from_*representation
fcd8101Updated pynormaliz to 1.15
19d8539Polyhedron_normaliz: In verbose mode, write out normaliz format files
4cfcd14_normaliz_format: Fix last change
ea7a6b8Polyhedron_normaliz: Prepare classes for work over field extensions

comment:7 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:8 follow-up: Changed 2 years ago by mkoeppe

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 2 years ago by mkoeppe

  • Description modified (diff)

comment:10 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:11 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:12 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:13 Changed 2 years ago by jipilab

  • Dependencies changed from #25090 to #25090, #20181

comment:14 in reply to: ↑ 8 Changed 2 years ago by jipilab

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?

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 2 years ago by git

  • Commit changed from ea7a6b83b235f76a0c98930d9d1aa42991ed6d7d to 13d27d4d037d6009c4f0f503d42a71a76aeacb78

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

26dc3e4Added option for embedded NumberField
db778eeMade it possible to deal with complex number fields
13d27d4Merge remote-tracking branch 'trac/public/algnum_to_numberfield' into t/25097/public/25097/qnormaliz-algebraic

comment:16 Changed 2 years ago by git

  • Commit changed from 13d27d4d037d6009c4f0f503d42a71a76aeacb78 to 5af97b3d83e504ee9b5d50d9e7e14b737c0cf73f

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

5af97b3Polyhedron_normaliz: Separate constructors for QQ, general fields (no cnaceling)

comment:17 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:18 Changed 2 years ago by mkoeppe

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 2 years ago by git

  • Commit changed from 5af97b3d83e504ee9b5d50d9e7e14b737c0cf73f to d2aee02648bb870af22bd24f910f786275bd807b

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

0f325c0Polyhedron: Allow SR input for normaliz
aa22d44Polyhedron_normaliz._nmz_result: New, use it instead of directly calling PyNormaliz.NmzResult
7e4f1acPolyhedron_normaliz: _init_from_Vrepresentation: Coerce to numberfield
0ba9f6bPolyhedron_normaliz: Pass number field to QNormaliz
d2aee02_init_from_normaliz_data: Separate implementations for QNormaliz, Normaliz

comment:20 Changed 2 years ago by git

  • 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
a9045bfAdded tests and made them pass
09df646Merge remote-tracking branch 'trac/public/algnum_to_numberfield' into t/25097/public/25097/qnormaliz-algebraic
01d7cfclists of lists of lists of algebraic numbers, actually

comment:21 Changed 2 years ago by git

  • Commit changed from 01d7cfc32446240c45371484acd6c12f625ca289 to d7ecb03da0437d90e76f77642aef94376a4aa619

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

d7ecb03Temporary build scripts for normaliz, pynormaliz, pyqnormaliz

comment:22 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:23 Changed 2 years ago by mkoeppe

  • Keywords IMA-PolyGeom added

comment:24 Changed 2 years ago by mkoeppe

  • Description modified (diff)

comment:25 Changed 2 years ago by git

  • Commit changed from d7ecb03da0437d90e76f77642aef94376a4aa619 to 79bda61224ade0f7003b17e6539c2c2f4ce451f8

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

8f675adAdd pyqnormaliz
79bda61Polyhedron_normaliz: Update for pyqnormaliz

comment:26 follow-up: Changed 2 years ago by 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?

comment:27 in reply to: ↑ 26 ; follow-up: Changed 2 years ago by 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'

comment:28 in reply to: ↑ 27 ; follow-up: Changed 2 years ago by 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 debian-sid distribution, it seems that it was not necessary.

comment:29 Changed 2 years ago by moritz

  • Description modified (diff)

comment:30 Changed 2 years ago by moritz

  • Description modified (diff)

comment:31 follow-up: Changed 2 years ago by jipilab

After installing autotools and automake I get the following error message:

[normaliz] Installing E-ANTIC...
[normaliz] From https://github.com/mkoeppe/e-antic
[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/E-ANTIC_source/e-antic'
[normaliz] configure: error: cannot find install-sh, 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 2 years ago by jipilab

... Problem solved by gh-sebasguts: there was an old config file lying around. Deleting it solved the issue.

comment:33 Changed 2 years ago by moritz

works flawlessly for me!

comment:34 in reply to: ↑ 28 Changed 2 years ago by gh-TimmyChan

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 debian-sid 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 apt-get install libssl-dev. After that I no longer have the error curl: (1) Protocol "https" not supported...

Last edited 2 years ago by gh-TimmyChan (previous) (diff)

comment:35 in reply to: ↑ 31 Changed 2 years ago by gh-TimmyChan

Replying to jipilab:

After installing autotools and automake I get the following error message:

[normaliz] Installing E-ANTIC...
[normaliz] From https://github.com/mkoeppe/e-antic
[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/E-ANTIC_source/e-antic'
[normaliz] configure: error: cannot find install-sh, 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 autotools-dev and automake and libtool and installation completed on Ubuntu 16.04LTS.

Last edited 2 years ago by gh-TimmyChan (previous) (diff)

comment:36 Changed 2 years ago by jipilab

There is a link error in my installation:

sage: import PyQNormaliz_cpp
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-10-4c34a88d49d7> 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 follow-ups: Changed 2 years ago by gh-sebasguts

While this solution works for jipilab, it does not for gh-TimmyChan?. 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 2 years ago by gh-TimmyChan

Replying to gh-sebasguts:

While this solution works for jipilab, it does not for gh-TimmyChan?. 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 21 months ago by Winfried

  • Description modified (diff)

comment:40 Changed 21 months ago by Winfried

Since version 3.6.0 QNormaliz and PyQNormaliz are included in the Normaliz distribution.

comment:41 Changed 19 months ago by git

  • Commit changed from 79bda61224ade0f7003b17e6539c2c2f4ce451f8 to de64a4ad06db3f26bce3b535e7d0022023097b77

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

855a2c0build/pkgs/normaliz/spkg-install: Only warn if git fetch/pull fails (airplane mode)
5829bc0src/sage/geometry/polyhedron/backend_normaliz.py: Add #optional - pyqnormaliz
de64a4abuild/pkgs/normaliz/dependencies: Depend on gc (dependency of our flint copy)

comment:42 Changed 19 months ago by mkoeppe

  • Description modified (diff)
  • Milestone changed from sage-8.2 to sage-8.4

comment:43 Changed 19 months ago by git

  • Commit changed from de64a4ad06db3f26bce3b535e7d0022023097b77 to 855391c03c327144d78c8a3dd7258e112c182c59

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

976b93fUpdate normaliz to 3.6.3, pynormaliz to 1.19
e36973abuild/pkgs/normaliz/spkg-install: Update comment
98d5bd3Merge remote-tracking branch 'trac/develop' into t/25090/public/pynormaliz1.17_normaliz3.6.1
aa71e25Merge remote-tracking branch 'trac/develop' into t/25090/public/pynormaliz1.17_normaliz3.6.1
7907f7aPolyhedron_normaliz: Fix: INPUT: items generally do not end with a period
1e20ef2Polyhedron_normaliz._make_normaliz_cone: Add doctest
f3d11a3Polyhedron_normaliz._init_from_normaliz_data: Add doctest
36d5473src/sage/geometry/polyhedron/backend_normaliz.py: Add # optional - pynormaliz
e852ffaMerge branch 't/25090/public/pynormaliz1.17_normaliz3.6.1' into t/25097/public/25097/qnormaliz-algebraic
855391cnormaliz, pynormaliz, pyqnormaliz: Update build scripts to use releases

comment:44 Changed 19 months ago by mkoeppe

Updated this branch to use the current released versions of Normaliz from #25090 and PyQNormaliz from pypi.

comment:45 Changed 19 months ago by mkoeppe

  • Description modified (diff)

comment:46 Changed 19 months ago by git

  • Commit changed from 855391c03c327144d78c8a3dd7258e112c182c59 to e34e10b8310ed9fa511a8c0f9d592da8d3785d0b

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

e34e10bPolyhedron_normaliz._nmz_result: Use the PyQNormaliz conversion handlers

comment:47 Changed 19 months ago by mkoeppe

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 19 months ago by Winfried

Could you give more specific information?

comment:49 Changed 19 months ago by mkoeppe

  • Description modified (diff)
sage: x = polygen(ZZ); P = Polyhedron(vertices=[[sqrt(2)], [AA.polynomial_root(x^3-2, 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.38e-16]
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.38e-16]', '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 follow-up: Changed 19 months ago by Winfried

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.57e-5]

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 19 months ago by mkoeppe

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 19 months ago by git

  • Commit changed from e34e10b8310ed9fa511a8c0f9d592da8d3785d0b to b6a16657020f71a1ffe0f4011c02a263f1128f87

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

b6a1665Polyhedron_normaliz._nmz_result: Fix up NumberfieldElementHandler

comment:53 Changed 19 months ago by mkoeppe

x = polygen(ZZ); P = Polyhedron(vertices=[[sqrt(2)], [AA.polynomial_root(x^3-2, RIF(0,3))]], backend='normaliz', verbose=True); P._Hrepresentation
.... segfault ...

Some memory corruption involving the gc, it seems

comment:54 Changed 19 months ago by git

  • Commit changed from b6a16657020f71a1ffe0f4011c02a263f1128f87 to ec5f5c9249da26235725d166d897514787bf9992

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

ec5f5c9Polyhedron_normaliz._normaliz_format: Update code that puts 'number_field' right after 'amb_space'

comment:55 Changed 19 months ago by git

  • Commit changed from ec5f5c9249da26235725d166d897514787bf9992 to 9669a6f17b687480bd94584b4cb5d437d4c34767

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

9669a6fPolyhedron_normaliz: If base_ring is already a number field, use it

comment:56 Changed 19 months ago by git

  • Commit changed from 9669a6f17b687480bd94584b4cb5d437d4c34767 to 3b454e23e9d0b8259135d3fbe5607985e9ff0fc4

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

2b149b5Polyhedron_normaliz._is_zero etc.: Add, copied from Polyhedron_field
3b454e2polytopes library: Add tests with backend='normaliz'

comment:57 Changed 19 months ago by mkoeppe

  • Dependencies changed from #25090, #20181 to #25090, #20181, #25171
  • Description modified (diff)

comment:58 Changed 19 months ago by git

  • Commit changed from 3b454e23e9d0b8259135d3fbe5607985e9ff0fc4 to eb6646e889bb9a706192726000cc7a0ad0c78811

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

46dfa08Add missing build/pkgs/pyqnormaliz/checksums.ini file
8b4deccExperimental upgrade of FLINT to 2.5.2+trunk-2018-09-18-e12c83de3e100a113dbc030894d89f67dfe83cd4; remove patches
2e3fe7dMerge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormaliz-algebraic
41f6f98build/{normaliz,pyqnormaliz}/spkg-install: Use FLINT from #25171, use sage arb, get rid of enfnormaliz/ install hierarchy
13089b4build/pkgs/flint/checksums.ini: Update for using github release tarball
eb6646eMerge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormaliz-algebraic

comment:59 in reply to: ↑ 37 Changed 19 months ago by mkoeppe

Replying to gh-sebasguts:

While this solution works for jipilab, it does not for gh-TimmyChan?. 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 19 months ago by git

  • Commit changed from eb6646e889bb9a706192726000cc7a0ad0c78811 to 95b5bdd0a3ca6a6fc2cbedfed2ef4f2062b7bcf0

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

1ccf632polytopes.snub_cube: Add keyword arg verbose
25e64fcpolytopes.snub_dodecahedron: Add keyword arg verbose, mark pyqnormaliz doctest 'not tested - very long time'
7dfd8a5polytopes.six_hundred_cell: Fix wrong doctest
95b5bddpolytopes.snub_cube: Add keywords exact, base_ring. Add pyqnormaliz doctest

comment:61 Changed 19 months ago by git

  • Commit changed from 95b5bdd0a3ca6a6fc2cbedfed2ef4f2062b7bcf0 to 3d8d078f7592dd4063d56d6459d64cfe015f8bbc

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

1b977dcPolyhedron_normaliz: Fix a doctest
4eaec75Polyhedron_normaliz: Sort keys in verbose output for predictable doctests
7749acePolyhedron_normaliz: Guard against deadly exceptions in *_handler
a692e85Polyhedron_QQ_normaliz._init_from_normaliz_data: Add doctests
3040b2dPolyhedron_normaliz: Refactor using format_number_field_data, passing normaliz_field through _init_from_normaliz_data
d56e0d4Polyhedron_normaliz: Refactor, passing normaliz_field through _init_from_normaliz_cone.
85166c3Polyhedron_normaliz: New init arg, normaliz_field. Fix _init_from_normaliz_data doctest
bb0f73dPolyhedron_normaliz: Extend doctest
3d8d078Polyhedron_normaliz: Switch between Normaliz, QNormaliz based on normaliz_field, rather than class

comment:62 Changed 19 months ago by mkoeppe

  • Description modified (diff)

comment:63 Changed 19 months ago by git

  • Commit changed from 3d8d078f7592dd4063d56d6459d64cfe015f8bbc to ad26e475628de592be3d6f878c187c9e34b5dc8a

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

ad26e47PyQNormaliz: Update to 1.2

comment:64 Changed 19 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)
  • Status changed from new to needs_review

comment:66 Changed 18 months ago by git

  • Commit changed from ad26e475628de592be3d6f878c187c9e34b5dc8a to 725ea5baae0cc2922baa0480e4c8cd7d88c9239d

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

725ea5bMerge branch 'trac/develop' into t/25097/public/25097/qnormaliz-algebraic

comment:67 Changed 18 months ago by git

  • Commit changed from 725ea5baae0cc2922baa0480e4c8cd7d88c9239d to bd3c990559c1531ecf14d98ba7b009d26504a05e

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

3881007PyQNormaliz: Update to 1.3
bd3c990sage.geometry.polyhedron.constructor: Update error message in doctest

comment:68 follow-up: Changed 12 months ago by vdelecroix

I think we should decouple: upgrading Normaliz and interfacing Normaliz.

comment:69 Changed 12 months ago by vdelecroix

  • Dependencies changed from #25090, #20181, #25171 to #25090, #20181, #25171, #27682

I opened #27682

comment:70 Changed 12 months ago by vdelecroix

  • Dependencies changed from #25090, #20181, #25171, #27682 to #20181, #27682
  • Description modified (diff)
  • Milestone changed from sage-8.4 to sage-8.8

comment:71 in reply to: ↑ 68 Changed 12 months ago by jipilab

Replying to vdelecroix:

I think we should decouple: upgrading Normaliz and interfacing Normaliz.

+1

comment:72 Changed 12 months ago by vdelecroix

I think it would be good to split this ticket into two tickets:

  1. Make backend='normaliz' accepts input being in a common embedded real number field. That is the following should work
    sage: 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 that backend='normaliz' is actually automatically chosen if available
    sage: 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 database polytopes..
  2. (follow-up 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 12 months ago by vdelecroix

  • Status changed from needs_review to needs_work

comment:74 Changed 12 months ago by git

  • Commit changed from bd3c990559c1531ecf14d98ba7b009d26504a05e to 2f3c42c418acfd67250f3db2b517cb125dc45a5b

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

61e4df9Fix docstring even more
9255f2cpyflakes
9a06e8eMerge branch 'public/algnum_to_numberfield' of git://trac.sagemath.org/sage into t/20181/public/algnum_to_numberfield
f8a2d52Merge branch 't/20181/public/algnum_to_numberfield' into t/25097/public/25097/qnormaliz-algebraic
46fcf67Merge tag '8.8.beta3' into t/25171/experimental_flint_upgrade_package_tracking_trunk
b320692Merge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormaliz-algebraic
76d7db1Add patch
c3fd47fMerge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormaliz-algebraic
0c93e30Add FLINT patch install_name_tool_destdir.patch
2f3c42cMerge branch 't/25171/experimental_flint_upgrade_package_tracking_trunk' into t/25097/public/25097/qnormaliz-algebraic

comment:75 Changed 12 months ago by mkoeppe

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 follow-up: Changed 12 months ago by git

  • Commit changed from 2f3c42c418acfd67250f3db2b517cb125dc45a5b to 7c3d64c8a12275511f3d3875d37f48d3cb6857ec

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

7c3d64cFix up for exact snub cube construction

comment:77 in reply to: ↑ 76 Changed 12 months ago by jipilab

Replying to git:

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

7c3d64cFix up for exact snub cube construction

Just a small heads-up about #26340 that will conflict with this line.

comment:78 Changed 12 months ago by mkoeppe

#27716 has all the backend work that does not depend on the algebraic stuff.

comment:79 follow-up: Changed 12 months ago by mkoeppe

I will next redo this ticket on top of #27716, #26340, #27682, #20181.

comment:80 Changed 12 months ago by git

  • Commit changed from 7c3d64c8a12275511f3d3875d37f48d3cb6857ec to ec695ac88bfb272dc05444a74579da8fdc0683ab

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

57a9d10add exact option to snub_cube
8703f7ffix snub cube example in polyhedra_tutorial.rst
7618ea9package e-antic
d752708upgrade normaliz
4eb02f8fix doctest in backend_normaliz.py
56a903eMerge branches 't/27716/public/normaliz-backend-refactoring', 't/26340/public/26340', 't/27682/27682' and 't/20181/public/algnum_to_numberfield' into t/25097/public/normaliz-3.7.0-algebraic
915cb46backend_normaliz: algebraic polyhedra
3a6e39dAdd library polytope tests for algebraic polytopes
c7aa2efchange use of PyQNormaliz to PyNormaliz
ec695acUse _number_field_triple instead of _format_number_field_data

comment:81 in reply to: ↑ 79 Changed 12 months ago by mkoeppe

Replying to mkoeppe:

I will next redo this ticket on top of #27716, #26340, #27682, #20181.

Done. Replaced the branch. Many doctests fail. Needs fixing: https://github.com/Normaliz/PyNormaliz/issues/37

comment:82 Changed 12 months ago by git

  • Commit changed from ec695ac88bfb272dc05444a74579da8fdc0683ab to a7e3af33495b528418de2af88d8b5b0eedc9d7dd

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

9732605Fix doctests
b7c73d1nfelem_handler: Protect against coordinate vectors with too many elements. Handle unhandled rationals
a7e3af3Fixup handling of number_field in normaliz data

comment:83 Changed 12 months ago by mkoeppe

All tests work now except for one that needs a fix for this: https://github.com/Normaliz/PyNormaliz/issues/41

comment:84 Changed 12 months ago by mkoeppe

  • Dependencies changed from #20181, #27682 to #27716, #26340, #27682, #20181

comment:85 Changed 12 months ago by mkoeppe

With Normaliz master (unreleased), all tests pass except for the broken snub cube example from #26340.

comment:86 Changed 12 months ago by git

  • Commit changed from a7e3af33495b528418de2af88d8b5b0eedc9d7dd to 19bb5d3ca9b2fcbec6f040ebc1f527689f104d74

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

3ce403dChange optional - pyqnormaliz to pynormaliz
b370268pyflakes fixes
328a04a_nmz_result: Add doctest
ee5ff41Mark newest doctest # optional - pynormaliz
b60f653Merge branch 't/27716/public/normaliz-backend-refactoring' into t/25097/public/normaliz-3.7.0-algebraic
05867b4Merge tag '8.8.beta3' into t/26340/public/26340
39c3c67Fix construction of snub_cube data
f52802fMerge branch 't/26340/public/26340' into t/25097/public/normaliz-3.7.0-algebraic
19bb5d3Add snub_cube test with normaliz

comment:87 Changed 11 months ago by git

  • Commit changed from 19bb5d3ca9b2fcbec6f040ebc1f527689f104d74 to 706e79648c58b2f9f69b5b9cc667a0c732096795

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

e70d4a5snub_cube: Change default back to exact=False
706e796Merge branch 't/26340/public/26340' into t/25097/public/normaliz-3.7.0-algebraic

comment:88 Changed 11 months ago by git

  • Commit changed from 706e79648c58b2f9f69b5b9cc667a0c732096795 to e3bbecda2f444ef6c21938a8bef3e4bd80095cb2

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

13d8c44docstring fix
e3bbecdMerge branch 't/26340/public/26340' into t/25097/public/normaliz-3.7.0-algebraic

comment:89 Changed 11 months ago by git

  • Commit changed from e3bbecda2f444ef6c21938a8bef3e4bd80095cb2 to 3ad690512e703b8868c36fa890cb5a477c5e0659

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

fb459b4Update e-antic to 0.1.3b0 and normaliz to 3.7.1
3ad6905Update pynormaliz to 2.1

comment:90 Changed 11 months ago by git

  • 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 11 months ago by git

  • Commit changed from e3bbecda2f444ef6c21938a8bef3e4bd80095cb2 to 24a87469967c0d47967c5214e2256cae7147b00b

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

eaa4584package e-antic 0.1.3b0
e5c669dupgrade normaliz
14d6c83Update normaliz to 3.7.1, pynormaliz to 2.1
64f1f19fix doctest in backend_normaliz.py
59e069bMerge branch 't/27682/27682' into t/25097/public/normaliz-3.7.0-algebraic
24a8746Update error message in doctest for new PyNormaliz

comment:92 Changed 11 months ago by mkoeppe

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Summary changed from Algebraic polyhedra with QNormaliz / e-antic to Algebraic polyhedra with Normaliz / e-antic

Ready for review. Builds and tests OK on macOS Mojave.

comment:93 Changed 11 months ago by jipilab

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 11 months ago by git

  • Commit changed from 24a87469967c0d47967c5214e2256cae7147b00b to f232c6cec018358b144f0f8b9541ae7b2fab8699

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

160dd88Merge branch 't/27682/27682' into t/25091/public/some_normaliz_features
78c2de4Merge branch 't/27716/public/normaliz-backend-refactoring' into t/25091/public/some_normaliz_features
41bd968Fix doctest error message - NormalizError
7662077Fixup merge
885ef9eUse EhrhartQuasiPolynomial
313fee1Fix doctest of hilbert_series
7905903Pass grading as a matrix
9249ddaUse _nmz_result instead of calling NmzResult directly
ec96615Merge branch 't/25091/public/some_normaliz_features' into t/25097/public/normaliz-3.7.0-algebraic
f232c6c_nmz_result already constructs rationals

comment:95 Changed 11 months ago by mkoeppe

  • Dependencies changed from #27716, #26340, #27682, #20181 to #27716, #26340, #27682, #20181, #25091

Merged #25091, works fine after minor changes.

comment:96 Changed 11 months ago by git

  • Commit changed from f232c6cec018358b144f0f8b9541ae7b2fab8699 to 812395cd57129ae1832562b23950ead1a1e6d97f

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

5d92502Add checks for real embedded base rings, improve error messages
6e2991bbackend_normaliz: Prepare refactoring of V, H data conversion
812395cRefactor using Polyhedron_normaliz._compute_nmz_data_lists_and_field

comment:97 Changed 11 months ago by git

  • Commit changed from 812395cd57129ae1832562b23950ead1a1e6d97f to 1dd5b3c3ce8cc2e23702188c035eadd2cbec406c

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

3d949a2Upgrade normaliz to 3.7.2, pynormaliz to 2.2
c1169c3Merge branch 't/27731/public/27731' into t/25097/public/normaliz-3.7.0-algebraic
8aee20fsnub_dodecahedron: Enable doctest
1dd5b3csnub_dodecahedron: Update tests, doc string

comment:98 Changed 11 months ago by mkoeppe

  • Dependencies changed from #27716, #26340, #27682, #20181, #25091 to #27716, #26340, #27682, #20181, #25091, #27731

comment:99 Changed 11 months ago by git

  • Commit changed from 1dd5b3c3ce8cc2e23702188c035eadd2cbec406c to ac4551155be9dec669518499c2fd6c6d563c9a31

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

ac45511Fix for tuples in H and V representations

Changed 11 months ago by jipilab

An error in convex hull?

comment:100 Changed 11 months ago by mkoeppe

That error was caused by using an old PyNormaliz?.

comment:101 Changed 11 months ago by git

  • Commit changed from ac4551155be9dec669518499c2fd6c6d563c9a31 to 2c3ea3c33f79e5fce5875efc8e6f1c474f907802

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

b488911Add build/pkgs/pynormaliz/spkg-check
5d01388Upgrade pynormaliz to 2.4
2c3ea3cMerge branch 't/27731/public/27731' into t/25097/public/normaliz-3.7.0-algebraic

comment:102 Changed 11 months ago by git

  • Commit changed from 2c3ea3c33f79e5fce5875efc8e6f1c474f907802 to 59f320b2fb7aa1ff552bf0d76dec5a4d48d1c5d2

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

a4e5977Upgrade pynormaliz to 2.5
59f320bMerge branch 't/27731/public/27731' into t/25097/public/normaliz-3.7.0-algebraic

comment:103 Changed 11 months ago by git

  • Commit changed from 59f320b2fb7aa1ff552bf0d76dec5a4d48d1c5d2 to ec2646dc2f2389af7bf09a364152920ecba7b9be

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

59cc7e0Fix Error messages
a4f01d4Fixed typo and added require package
fbd3dceremoved is_package_installed
8ceee84Change TypeError for RuntimeError
0da067fFixed doctests and added author name
d7122d7pyflakes
63a80b6Forgot an import
d8716e3Refactor NmzResult
08b315bFixed infinite recursion
ec2646dMerge branch 't/25091/public/some_normaliz_features' into t/25097/public/normaliz-3.7.0-algebraic

comment:104 follow-up: Changed 11 months ago by jipilab

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[i-1] * 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 11 months ago by mkoeppe

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 11 months ago by mkoeppe

This should be a separate ticket

comment:107 Changed 11 months ago by git

  • Commit changed from ec2646dc2f2389af7bf09a364152920ecba7b9be to 978765bd816698be8d73ed04db26a2206ea8d38f

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

292e01fbackend_normaliz: py3 fix
d44a00dbackend_normaliz: Canonicalize sort order of fields in normaliz files
dd9a81bMerge tag '8.8.beta4' into t/27731/public/27731
9b3a5baFix error message in doctest for new pynormaliz version
0a8c6f8Merge branch 't/27782/1_pynormaliz_doctest_failing_in_src_sage_geometry_polyhedron_backend_normaliz_py' into t/27731/public/27731
978765bMerge branch 't/27731/public/27731' into t/25097/public/normaliz-3.7.0-algebraic

comment:108 Changed 11 months ago by jipilab

  • 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 11 months ago by git

  • Commit changed from 978765bd816698be8d73ed04db26a2206ea8d38f to a8f307e454cd16ef20850b99057f22c368629e4e

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

da05706Fix forgotten optional package test
ec949bdMerge branch 't/25091/public/some_normaliz_features' into t/25097/public/normaliz-3.7.0-algebraic
a8f307eMerge tag '8.8.beta5' into t/25097/public/normaliz-3.7.0-algebraic

comment:110 Changed 11 months ago by mkoeppe

Merged in latest version of #25091 and current beta.

comment:111 Changed 11 months ago by mkoeppe

  • Dependencies changed from #27716, #26340, #27682, #20181, #25091, #27731 to #25091

comment:112 Changed 11 months ago by git

  • Commit changed from a8f307e454cd16ef20850b99057f22c368629e4e to ed6fcbf1ab93216403c74448b275fcdf74b12ae5

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

ca7c25csage/geometry/polyhedron/parent.py: Change wording of error message
7a0886aFix doctest
ed6fcbfUpdate polyhedron tutorial

comment:113 Changed 11 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Setting it back to needs_review to get a fresh patchbot run.

comment:114 Changed 11 months ago by git

  • Commit changed from ed6fcbf1ab93216403c74448b275fcdf74b12ae5 to 4bd7635c5506c2de2df92f5674fab61adf5f698c

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

4bd7635Add more optional - pynormaliz

comment:115 in reply to: ↑ 104 ; follow-up: Changed 11 months ago by 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?

comment:116 Changed 11 months ago by git

  • Commit changed from 4bd7635c5506c2de2df92f5674fab61adf5f698c to b484af77e078358d50be4ed0a4eba92df7872c8d

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

0ba5515Polyhedron_normaliz._convert_to_pynormaliz: Rename from _convert_to_Qnormaliz, make static, add doctest, use int() instead of long()
b484af7Update doctests for int/long, py3

comment:117 Changed 11 months ago by git

  • Commit changed from b484af77e078358d50be4ed0a4eba92df7872c8d to 99e12c15bf28965b61487dd99b8c720aa00c09bc

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

99e12c1Polyhedron_normaliz._number_field_triple: Add doctest

comment:118 Changed 11 months ago by git

  • 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 11 months ago by git

  • Commit changed from 4d82f5f5fd075aeea0be7965853fd3b557481166 to fcf2c353f0e99ca2e72a6da98d3b6e43689229fc

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

ef280d1Fix triple_colon
d03c531py3: Change iteritems to items
fcf2c35py3: Get rid of has_key; avoid unnecessary copy

comment:120 Changed 11 months ago by mkoeppe

  • Dependencies changed from #25091 to #25091, #27731

comment:121 Changed 11 months ago by git

  • Commit changed from fcf2c353f0e99ca2e72a6da98d3b6e43689229fc to abb7b02fd0e2866b1cc5627d9046f89074ee7b09

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

abb7b02More optional - pynormaliz

comment:122 in reply to: ↑ 115 Changed 11 months ago by jipilab

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 well-defined, 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 follow-up: Changed 11 months ago by jipilab

  • 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 11 months ago by jipilab

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 11 months ago by git

  • Commit changed from abb7b02fd0e2866b1cc5627d9046f89074ee7b09 to ca71cc29875892438eb8d77393b5ec1635f015b0

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

ad4d739Merge tag '8.8.beta6' into t/25097/public/normaliz-3.7.0-algebraic
ca71cc2polyhedra_tutorial: Fix error messages in doctests

comment:126 Changed 11 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Fixed the doctest and merged in the newest beta.

comment:127 Changed 10 months ago by jipilab

  • Status changed from needs_review to needs_work

There is 1 doctest failing because of #21161. :-/

comment:128 Changed 10 months ago by jipilab

  • Reviewers set to Jean-Philippe Labbé

comment:129 Changed 10 months ago by jipilab

Now with #27965, we may get a conflict in snub dodecahedron. I was not quick enough to warn about it on the ticket #27965 about the current one...

comment:130 Changed 10 months ago by jipilab

  • 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 10 months ago by embray

  • Milestone changed from sage-8.8 to sage-8.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 10 months ago by git

  • Commit changed from ca71cc29875892438eb8d77393b5ec1635f015b0 to 4258704205e49d5786850ef657465a7e3185f0c0

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

28b8dbdMerge tag '8.8.rc1' into t/25097/public/normaliz-3.7.0-algebraic
6fcdf05trac 27965: py3 fix in geometry/polyhedron/library.py
3d46bb2Merge branch 't/27965/27965' into t/25097/public/normaliz-3.7.0-algebraic
4258704Fix doctests for changed embedded numberfield repr

comment:133 Changed 10 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:134 Changed 10 months ago by jipilab

  • Reviewers changed from Jean-Philippe Labbé to Vincent Delecroix, Jean-Philippe 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 9 months ago by jipilab

ping!

Any last comments Vincent?

comment:136 Changed 9 months ago by chapoton

  • Status changed from needs_review to needs_work

doctest failure in /library with py3

comment:137 Changed 9 months ago by git

  • Commit changed from 4258704205e49d5786850ef657465a7e3185f0c0 to f94bf376ea7c391fd779587968738af23c486b07

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

23b1cbeMerge tag '8.9.beta1' into t/25097/public/normaliz-3.7.0-algebraic
f94bf37snub_dodecahedron: Disable doctests for broken floating point computation

comment:138 Changed 9 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:139 Changed 9 months ago by chapoton

The last changes looks reasonable to me.

comment:140 Changed 9 months ago by jipilab

  • 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 follow-ups if need be.

comment:141 Changed 9 months ago by vbraun

  • Branch changed from public/25097/qnormaliz-algebraic to f94bf376ea7c391fd779587968738af23c486b07
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.