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

Fixes to hash functions for lists #5796

Merged
merged 4 commits into from
Sep 18, 2024
Merged

Fixes to hash functions for lists #5796

merged 4 commits into from
Sep 18, 2024

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Sep 14, 2024

Limit scope of list hashing to permutations acting by permuting, thus avoiding problems when acting with matrices on lists of vectors. This fixes #5786

Also acknowledge, in test, related error reported in support, that already had been fixed in master

Use list hashing for lists of integers, not lists of vectors.

This fixes gap-system#5786
This change is commented out, as a change in main branch already resolved it, but code is preserved
in case needed at later time:
This fixes an issue reported in support by Leonard Soicher on Sep 13, 2024.
@hulpke hulpke added kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Sep 14, 2024
@hulpke hulpke requested a review from dimpase September 14, 2024 23:20
@dimpase
Copy link
Member

dimpase commented Sep 15, 2024

OK, this fixes #5786
I tried to run

gg:=SpecialUnitaryGroup(4,2);
hl:=Z(2)*[
          [0,0,1,0],
          [1,1,0,0],
          [0,1,0,1],
          [0,1,1,0],
          [1,1,0,1]];
o216:=Orbit(gg,Set(hl),OnSets);

which passes with this PR, and fails without.
Should we add this piece to the tests?

PS. Oh, it's there already. Sorry for noise. All good.

@dimpase
Copy link
Member

dimpase commented Sep 15, 2024

if there is a "rule" that there should be no spaces around <> then it's inconsistently applied in this patch.

@hulpke hulpke merged commit fddffa2 into gap-system:master Sep 18, 2024
33 checks passed
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Oct 9, 2024
Backport gap-system/gap#5796 because it's
causing failures in the SageMath test suite when dev-gap/grape also
happens to be installed.

Thanks to Dima Pasechnik for the pointer.

Signed-off-by: Michael Orlitzky <[email protected]>
orlitzky added a commit to orlitzky/sage that referenced this pull request Oct 12, 2024
Fixes gap-system/gap#5796

This is the patch I'm using on Gentoo, it combines the upstream fix
and one of its dependencies (without which the desired fix does not
apply). Without it, you'll get some test failures with gap-4.13.x
when GRAPE happens to be installed.
orlitzky added a commit to orlitzky/sage that referenced this pull request Oct 12, 2024
Fixes gap-system/gap#5796

This is the patch I'm using on Gentoo, it combines the upstream fix
and one of its dependencies (without which the desired fix does not
apply). Without it, you'll get some test failures with gap-4.13.x
when GRAPE happens to be installed.
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 20, 2024
    
Follow-up to:

* sagemath#37884
* sagemath#38169

With the four additional work items I mentioned in a comment on the
latter:

1. Everything has been rebased
2. There's a new feature to detect the polenta GAP package
3. The three failing simplicial sets tests have been marked `# needs
gap_package_polenta`
4. I backported gap-system/gap#5796 to the GAP
spkg so that the optional GRAPE tests will pass (untested).

Let's see what the CI has to say...
    
URL: sagemath#38804
Reported by: Michael Orlitzky
Reviewer(s): Dima Pasechnik, Enrique Manuel Artal Bartolo
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 23, 2024
    
Follow-up to:

* sagemath#37884
* sagemath#38169

With the four additional work items I mentioned in a comment on the
latter:

1. Everything has been rebased
2. There's a new feature to detect the polenta GAP package
3. The three failing simplicial sets tests have been marked `# needs
gap_package_polenta`
4. I backported gap-system/gap#5796 to the GAP
spkg so that the optional GRAPE tests will pass (untested).

Let's see what the CI has to say...
    
URL: sagemath#38804
Reported by: Michael Orlitzky
Reviewer(s): Dima Pasechnik, Enrique Manuel Artal Bartolo
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 26, 2024
    
Follow-up to:

* sagemath#37884
* sagemath#38169

With the four additional work items I mentioned in a comment on the
latter:

1. Everything has been rebased
2. There's a new feature to detect the polenta GAP package
3. The three failing simplicial sets tests have been marked `# needs
gap_package_polenta`
4. I backported gap-system/gap#5796 to the GAP
spkg so that the optional GRAPE tests will pass (untested).

Let's see what the CI has to say...
    
URL: sagemath#38804
Reported by: Michael Orlitzky
Reviewer(s): Dima Pasechnik, Enrique Manuel Artal Bartolo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Orbit() of a matrix group on sets of vectors stopped working in GAP 4.13
2 participants