Skip to content

Comments

Support bigint, bigintvec, and bigintmat#16

Merged
fingolfin merged 1 commit intogap-packages:masterfrom
jamesjer:master
Aug 26, 2025
Merged

Support bigint, bigintvec, and bigintmat#16
fingolfin merged 1 commit intogap-packages:masterfrom
jamesjer:master

Conversation

@jamesjer
Copy link
Contributor

@jamesjer jamesjer commented Jun 5, 2025

Fixes #15. This approach is a bit clunky in places. The use of an anonymous function with an unused parameter in ParseGapIntmatToSingBigintmat, for example, makes me cringe, but I couldn't find any other way of writing a bigintmat expression with literal elements. My hope is that those with more knowledge of GAP and Singular can help polish this up.

@jamesjer
Copy link
Contributor Author

jamesjer commented Jun 5, 2025

I expected this to work for all 4.x versions of Singular. I looked in the original Singular 4.0.0 release from December 2013, and saw bigintmat in use there. Perhaps the syntax used in this patch is only supported in newer versions of Singular? What version of Singular is used to run the tests?

@jamesjer
Copy link
Contributor Author

jamesjer commented Jun 6, 2025

Here's a version that works for me on Fedora 39, which has gap 4.12.2 and Singular 4.3.1p1. It appears that bigintvec was not introduced until Singular 4.4, so I have omitted mention of it from the tests. Probably there should be a separate test file that is executed only if Singular 4.4 is in use. This version of the patch almost works on Fedora 42, which has gap 4.14.0 and Singular 4.4.1, except the total number of inputs and outputs is 1156 instead of 1150. It would still be helpful to know what version of Singular is used by the CI.

@jamesjer
Copy link
Contributor Author

jamesjer commented Jun 6, 2025

Ah, interesting, all of the CI failures show the 1150 vs 1156 difference. I wonder why I'm getting 1150 on Fedora 39? Well, it's a dead distribution, so 1156 it is.

@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

❌ Patch coverage is 77.77778% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.54%. Comparing base (d9b3ee7) to head (a4dfb20).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
gap/singular.g 77.77% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
+ Coverage   71.49%   71.54%   +0.05%     
==========================================
  Files           2        2              
  Lines        1484     1536      +52     
==========================================
+ Hits         1061     1099      +38     
- Misses        423      437      +14     
Files with missing lines Coverage Δ
gap/singular.g 71.45% <77.77%> (+0.05%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamesjer
Copy link
Contributor Author

jamesjer commented Jun 6, 2025

Now I wonder if the check for '[' is really needed. I added that due to a weird failure on Fedora 39. Perhaps something was broken in either the gap package or the Singular package in Fedora 39. I'm going to try taking that out. If CI fails, I'll put it back.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks

@fingolfin fingolfin merged commit 1a11eb6 into gap-packages:master Aug 26, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for bigint, bigintvec, bigintmat

2 participants