Skip to content

Fix crash in ConvertToVectorRep for field size 1 - #6825

Open
fingolfin wants to merge 2 commits into
masterfrom
mh-ai/vector-rep-field-size-one
Open

fingolfin wants to merge 2 commits into
masterfrom
mh-ai/vector-rep-field-size-one

Conversation

@fingolfin

Copy link
Copy Markdown
Member

ConvertToVectorRep(v, 1) and CopyToVectorRep(v, 1) could pass the field check, since 1 is the zeroth power of every field size, and handed q = 1 to the kernel, which crashed. They now raise an error.

Assisted-by: Generative AI

ConvertToVectorRep(v, 1) and CopyToVectorRep(v, 1) could pass the
field check, since 1 is the zeroth power of every field size, and
handed q = 1 to the kernel, which crashed. They now raise an error.

Assisted-by: Generative AI
@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) topic: library release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Nov 10, 2026
Apply the fix for ConvertToVectorRep(v, 1) and CopyToVectorRep(v, 1)
to the HPC-GAP copy of vecmat.gi as well.

Assisted-by: Generative AI
@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.48%. Comparing base (ed07d9b) to head (4bafba0).
⚠️ Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (ed07d9b) and HEAD (4bafba0). Click for more details.

HEAD has 6 uploads less than BASE
Flag BASE (ed07d9b) HEAD (4bafba0)
10 4
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6825      +/-   ##
==========================================
- Coverage   79.94%   71.48%   -8.47%     
==========================================
  Files         685      681       -4     
  Lines      295944   294746    -1198     
  Branches     8672     8679       +7     
==========================================
- Hits       236599   210699   -25900     
- Misses      57533    82110   +24577     
- Partials     1812     1937     +125     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ThomasBreuer ThomasBreuer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Funny. Thanks.

(The documentation of LogInt says that the result is a positive integer.
Should we say that 0 is returned for n = 1, or should the input n = 1 better be forbidden?)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) kind: bug Issues describing general bugs, and PRs fixing them release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants