make sagelib work with singular>=4.3.2.p15 (future 4.4) #37492

antonio-rojas · 2024-02-27T21:21:18Z

Two major breaking changes:

The rp ordering is renamed to ip
hilb now returns a bigintvec object (which isn't even exposed in the C++ API, seems to be an instance of bigintmat)

This branch builds and works fine with 4.3.2.p15, but I have no idea how to make it work with both old and new singular, suggestions welcome.

antonio-rojas · 2024-02-27T21:21:36Z

cc @tornaria @kiwifb

tornaria · 2024-03-21T11:43:39Z

This is working fine for me and I'm releasing sagemath 10.3 on void linux with this patch and singular 4.3.2.p16 (with flint 3.1.2)

mkoeppe · 2024-03-21T16:28:20Z

best to remove "WIP:" from the title if it's ready

antonio-rojas · 2024-03-21T16:42:35Z

best to remove "WIP:" from the title if it's ready

I don't think this is ready unless we want to break support for older singular.

mkoeppe · 2024-03-21T16:58:53Z

Then probably "positive review" should be replaced by "needs work"

src/sage/libs/singular/singular.pyx

src/sage/libs/singular/singular.pxd

antonio-rojas · 2024-03-31T21:01:10Z

First attempt at making this work with older singular

antonio-rojas · 2024-04-01T06:49:22Z

src/sage/interfaces/singular.py

            polynomial ring, over a field, global ordering
            //   coefficients: ZZ/127
            //   number of vars : 3
-            //        block   1 : ordering rp
+            //        block   1 : ordering ip


I considered this, but do we really want to skip testing that the correct ordering is used here?

We really need a mechanism to support backward compatibility. Sprinkling examples with regular expressions to fix stuff like this is quite ugly in terms of documentation.

Similar problems we have right now with gap 4.13 (#37616, see also #37590 (comment)) and all over the place with results that have to be sorted to compare, etc.

For numerical noise there's the # tol tag which affects how the output of a doctest is compared. For order, I think a tag # random_order may be preferable to having to sort in the example. What could we do for this case?

An alternative is to add code in SingularElement._repr_() so the output of old singular is changed into the output of new singular (i.e. replace ordering rp with ordering ip). But this seems a bit hacky...

src/sage/libs/singular/decl.pxd

@@ -244,6 +244,7 @@ cdef extern from "singular/Singular/libsingular.h":
        ringorder_lp
        ringorder_dp
        ringorder_rp
+        ringorder_ip


tornaria · 2024-04-01T01:37:41Z

src/sage/rings/polynomial/term_order.py

 singular_name_mapping = {
    'lex'           : 'lp',
-    'invlex'        : 'rp',
+    'invlex'        : invlex_singular_name,


Seems to be the only place we need a different code for different versions of singular.

src/sage/rings/polynomial/multi_polynomial_libsingular.pyx

            polynomial ring, over a field, global ordering
            //   coefficients: ZZ/2[a]/(a^8+a^4+a^3+a^2+1)
            //   number of vars : 10
-            //        block   1 : ordering rp
+            //        block   1 : ordering ip


tornaria · 2024-04-01T01:39:37Z

src/sage/libs/singular/ring.pyx

+from sage.interfaces.singular import singular_version_number
+if int(singular_version_number()[0:3]) < 432 or (int(singular_version_number()[0:3]) == 432 and int(singular_version_number()[3:]) < 15):
+    order_dict["rp"] = ringorder_rp
+else:
+    order_dict["ip"] = ringorder_ip


seems unnecessary if we keep using ringorder_rp for the time being.

EDIT: wrong.. still need "rp" or "ip" 🤔

EDIT 2: Could just add both as in

"rp" : ringorder_rp, # this is deprecated "ip" : ringorder_ip,

As a matter of fact, ringorder_rp is an alias for ringorder_ip in old and in new singular. The only thing changed now is the name "rp" was changed to "ip".

tornaria · 2024-04-06T22:42:39Z

@antonio-rojas

antonio-rojas/sage@singular-4.4...tornaria:sage:singular

This is my attempt to support singular 4.3.2p10 and before. This is built on top of your first commit here, but I dropped the others.

add a doctest for bigintvec coercion (you implemented this but there's no test that will use this code)
fix the "arity" renumbering by printing the symbol instead of a number (seems nicer)
fix the compatibility for 4.3.2p10 by adding some #define and (only for "old" singular) patching the mapping tables so we use rp on singular side and patch output so it prints ordering ip even when using old singular (this solves your concern about testing the ordering).

I've tested with 4.3.2p10 (old) and 4.3.2p16 (new).

If you like the approach, feel free to take my commits into your PR, rebase at will. I don't make a PR to your PR since I am dropping some of your commits.

Note that I left out the stuff about noexcept; since #37667 will be merged in beta2, it seems easier to wait and fix it when rebasing on top of beta2.

tornaria · 2024-04-08T23:07:38Z

@antonio-rojas I've rebased my branch develop...tornaria:sage:singular on top of 10.4.beta2.

I adjusted the noexcept conflict in your commit, and my commits rebase cleanly.

github-actions · 2024-04-09T14:21:58Z

Documentation preview for this PR (built with commit d999b20; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

mkoeppe · 2024-04-11T06:38:41Z

I've tested this with:

build/pkgs/singular: Update to 4.4.0 #37570

The tests include a mixture of platforms where Singular is built from the SPKG or taken from the system.
All works well.
Thanks for preparing this!

src/sage/interfaces/singular.py

antonio-rojas · 2024-04-14T12:24:23Z

I modified the ordering string replacement to happen only in doctests. Otherwis it can be confusing to get a different output for Singular code when run from inside Sage.

mkoeppe · 2024-04-24T06:56:11Z

Tested again with #37570 (and some other upgrades) with the full CI Linux in https://github.com/mkoeppe/sage/actions/runs/8761823524, looking good.

mkoeppe · 2024-04-24T06:57:06Z

Is there anything else to do here? From my side, this would be a positive review.

antonio-rojas · 2024-04-24T06:58:43Z

Is there anything else to do here? From my side, this would be a positive review.

Not that I'm aware of

kiwifb · 2024-04-24T07:12:54Z

I have been including it in the latest beta release for sage-on-gentoo and everything looks fine.

vbraun · 2024-04-24T22:47:37Z

merge conflict

Co-authored-by: Gonzalo Tornaría <tornaria@cmat.edu.uy>

This reverts commit ad3aa56.

Function `hilb` now returns a `bigintvec` object and the coercion to a sage vector was implemented in the previous commit. Here we add a doctest for this coercion.

This makes the output independent of changes in the version of singular

Add two fallback compatibility `#define`s: - `ringorder_ip` - `BIGINTVEC_CMD` Also for old singular: - patch the term_order mappings to send `rp` to singular instead of `ip` - patch the display of a ring so it prints `ip` instead of `rp`

antonio-rojas · 2024-04-27T16:24:20Z

Rebased over beta 4

…4.4) Two major breaking changes: - The `rp` ordering is renamed to `ip` - `hilb` now returns a `bigintvec` object (which isn't even exposed in the C++ API, seems to be an instance of `bigintmat`) This branch builds and works fine with 4.3.2.p15, but I have no idea how to make it work with both old and new singular, suggestions welcome. URL: sagemath#37492 Reported by: Antonio Rojas Reviewer(s): Antonio Rojas, Gonzalo Tornaría, Matthias Köppe

Includes - [x] Singular/Singular#1205 ### 📝 Checklist  - [x] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies    - Depends on sagemath#37492 URL: sagemath#37570 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee

mkoeppe mentioned this pull request Mar 9, 2024

build/pkgs/singular: Update to 4.4.0 #37570

Merged

6 tasks

tornaria mentioned this pull request Mar 17, 2024

Do not canonicalize generators when building a permutation group from a libGAP group #37590

Merged

5 tasks

tornaria added the s: positive review label Mar 21, 2024

tornaria mentioned this pull request Mar 21, 2024

sagemath: update to 10.3. void-linux/void-packages#47712

Merged

antonio-rojas added s: needs work and removed s: positive review labels Mar 21, 2024

tornaria reviewed Mar 27, 2024

View reviewed changes

src/sage/libs/singular/singular.pyx Outdated Show resolved Hide resolved

src/sage/libs/singular/singular.pxd Outdated Show resolved Hide resolved

antonio-rojas force-pushed the singular-4.4 branch from 680283a to ad3aa56 Compare March 31, 2024 20:57

antonio-rojas changed the title ~~WIP: make sagelib work with singular>=4.3.2.p15 (future 4.4)~~ make sagelib work with singular>=4.3.2.p15 (future 4.4) Mar 31, 2024

antonio-rojas added s: needs review and removed s: needs work labels Mar 31, 2024

tornaria reviewed Apr 1, 2024

View reviewed changes

tornaria mentioned this pull request Apr 8, 2024

typeof(<bigintvec>) gives "?unknown type?" Singular/Singular#1213

Closed

mkoeppe approved these changes Apr 11, 2024

View reviewed changes

tornaria reviewed Apr 11, 2024

View reviewed changes

src/sage/interfaces/singular.py Show resolved Hide resolved

antonio-rojas force-pushed the singular-4.4 branch from 9667bdd to bb5b360 Compare April 13, 2024 09:40

mkoeppe added s: positive review and removed s: needs review labels Apr 24, 2024

vbraun added the s: needs work label Apr 24, 2024

antonio-rojas and others added 8 commits April 27, 2024 18:17

Adapt to singular 4.4 API changes

ec5ac91

Support singular older than 4.3.2.p15 too

1265e52

Remove noexcept keyword

f231930

Co-authored-by: Gonzalo Tornaría <tornaria@cmat.edu.uy>

Revert "Support singular older than 4.3.2.p15 too"

08db5bd

This reverts commit ad3aa56.

singular: add a doctest for bigintvec coercion

14586f1

Function `hilb` now returns a `bigintvec` object and the coercion to a sage vector was implemented in the previous commit. Here we add a doctest for this coercion.

singular: print arity as a symbol name

729e77d

This makes the output independent of changes in the version of singular

singular: compatibility for 4.3.2p10 and before

e04869f

Add two fallback compatibility `#define`s: - `ringorder_ip` - `BIGINTVEC_CMD` Also for old singular: - patch the term_order mappings to send `rp` to singular instead of `ip` - patch the display of a ring so it prints `ip` instead of `rp`

Only replace the Singular ordering string in doctests

d999b20

antonio-rojas force-pushed the singular-4.4 branch from 8f10ebc to d999b20 Compare April 27, 2024 16:22

antonio-rojas removed the s: needs work label Apr 27, 2024

vbraun merged commit 721fc79 into sagemath:develop May 2, 2024
11 of 13 checks passed

github-actions bot removed the s: positive review label May 2, 2024

mkoeppe added this to the sage-10.4 milestone May 2, 2024

antonio-rojas deleted the singular-4.4 branch May 25, 2024 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make sagelib work with singular>=4.3.2.p15 (future 4.4) #37492

make sagelib work with singular>=4.3.2.p15 (future 4.4) #37492

antonio-rojas commented Feb 27, 2024

antonio-rojas commented Feb 27, 2024

tornaria commented Mar 21, 2024

mkoeppe commented Mar 21, 2024

antonio-rojas commented Mar 21, 2024

mkoeppe commented Mar 21, 2024

antonio-rojas commented Mar 31, 2024

This comment was marked as outdated.

antonio-rojas Apr 1, 2024

tornaria Apr 3, 2024

This comment was marked as resolved.

tornaria Apr 1, 2024

This comment was marked as outdated.

tornaria Apr 1, 2024 •

edited

Loading

tornaria commented Apr 6, 2024

tornaria commented Apr 8, 2024

github-actions bot commented Apr 9, 2024 •

edited

Loading

mkoeppe commented Apr 11, 2024

antonio-rojas commented Apr 14, 2024

mkoeppe commented Apr 24, 2024

mkoeppe commented Apr 24, 2024

antonio-rojas commented Apr 24, 2024

kiwifb commented Apr 24, 2024

vbraun commented Apr 24, 2024

antonio-rojas commented Apr 27, 2024

make sagelib work with singular>=4.3.2.p15 (future 4.4) #37492

make sagelib work with singular>=4.3.2.p15 (future 4.4) #37492

Conversation

antonio-rojas commented Feb 27, 2024

antonio-rojas commented Feb 27, 2024

tornaria commented Mar 21, 2024

mkoeppe commented Mar 21, 2024

antonio-rojas commented Mar 21, 2024

mkoeppe commented Mar 21, 2024

antonio-rojas commented Mar 31, 2024

This comment was marked as outdated.

antonio-rojas Apr 1, 2024

Choose a reason for hiding this comment

tornaria Apr 3, 2024

Choose a reason for hiding this comment

This comment was marked as resolved.

tornaria Apr 1, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

tornaria Apr 1, 2024 • edited Loading

Choose a reason for hiding this comment

tornaria commented Apr 6, 2024

tornaria commented Apr 8, 2024

github-actions bot commented Apr 9, 2024 • edited Loading

mkoeppe commented Apr 11, 2024

antonio-rojas commented Apr 14, 2024

mkoeppe commented Apr 24, 2024

mkoeppe commented Apr 24, 2024

antonio-rojas commented Apr 24, 2024

kiwifb commented Apr 24, 2024

vbraun commented Apr 24, 2024

antonio-rojas commented Apr 27, 2024

tornaria Apr 1, 2024 •

edited

Loading

github-actions bot commented Apr 9, 2024 •

edited

Loading