Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various minor meataxe improvements #5957

Merged
merged 10 commits into from
Mar 21, 2025
Merged

Various minor meataxe improvements #5957

merged 10 commits into from
Mar 21, 2025

Conversation

fingolfin
Copy link
Member

  • meataxe: simplify SMTX_NewEqns
  • meataxe: use some MatrixObj features
  • meataxe: avoid a DefaultField call
  • meataxe: fix comment typos
  • meataxe: ensure more matrices are/stay compressed
  • meataxe: avoid repeatedly inverting the same matrix

Recently @fieker pointed out some small inputs for which the GAP meataxe takes unreasonable amounts of time.

While working on resolving these, I have made numerous small tweaks that also should be added. Since they are mostly local and hopefully uncontroversial I am submitting them hereby in a separate PR.

@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Mar 14, 2025
@fingolfin fingolfin requested a review from ThomasBreuer March 14, 2025 13:22
Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

If SMTX_NewEqns shall be restricted as proposed, the tests in tst/testinstall/meatauto.tst have to be adjusted.
However, since there are anyhow only very few tests for meatauto.gi, I do not know whether this is a good idea.

vec := [], # right-hand sides of system
failed := false, # flag to indicate inconsistent system
index := [], # index for row ordering
);
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a test in tst/testinstall/meatauto.tst where the arguments are a matrix and a vector.

Copy link
Member Author

Choose a reason for hiding this comment

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

I adjusted the test and added some new ones.

There is only one caller fort his which uses the first
argument variant. So just remove the superfluous code.
Instead pass the field to the internal helper SMTX_NilpotentBasis
Copy link
Contributor

@ThomasBreuer ThomasBreuer 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, thanks.

@fingolfin fingolfin enabled auto-merge (squash) March 20, 2025 14:09
for MTX.BasisModuleEndomorphisms and MTX.HomogeneousComponents
@fingolfin fingolfin merged commit 91a3c92 into master Mar 21, 2025
37 checks passed
@fingolfin fingolfin deleted the mh/meataxe branch March 21, 2025 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants