Skip to content

Hide GAP finiteness warnings during isomorphism check #38956

Merged
vbraun merged 2 commits intosagemath:developfrom
orlitzky:hide-gap-info-warnings
Dec 8, 2024
Merged

Hide GAP finiteness warnings during isomorphism check #38956
vbraun merged 2 commits intosagemath:developfrom
orlitzky:hide-gap-info-warnings

Conversation

@orlitzky
Copy link
Copy Markdown
Contributor

To fix #38889 and others like it, we hide the "#I Forcing finiteness test warning that can be printed by GAP if one of the groups is not (yet) known to be finite.

…ecks

GAP emits an "InfoWarning" message when it is checking for isomorphism
between two groups that are not known to be finite. This message is
only shown once, however, because after it checks for finiteness, it
knows. We have to keep this in mind during tests. For example,

  File "src/sage/groups/finitely_presented_named.py", line 485, in
  sage.groups.finitely_presented_named.AlternatingPresentation
  Failed example:
      A1.is_isomorphic(A2), A1.order()
  Expected:
      (True, 1)
  Got:
      #I  Forcing finiteness test
      (True, 1)

These warnings are "normal" and there's nothing for the user to do.
This commit hides them for the duration of the isomorphism check.

Fixes sagemath#38889
The warning "#I  Forcing finiteness test" should no longer be emitted
when checking for group isomorphism, so we remove it from the expected
output in a few places.

Fixes sagemath#38889
@github-actions
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Collaborator

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

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

LGTM.

@orlitzky
Copy link
Copy Markdown
Contributor Author

orlitzky commented Nov 17, 2024 via email

@tobiasdiez tobiasdiez added the p: CI fix merged before running CI tests label Nov 18, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 6, 2024
sagemathgh-38956: Hide GAP finiteness warnings during isomorphism check 
    
To fix sagemath#38889 and others like it,
we hide the `"#I  Forcing finiteness test` warning that can be printed
by GAP if one of the groups is not (yet) known to be finite.
    
URL: sagemath#38956
Reported by: Michael Orlitzky
Reviewer(s): David Coudert
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 8, 2024
sagemathgh-38956: Hide GAP finiteness warnings during isomorphism check 
    
To fix sagemath#38889 and others like it,
we hide the `"#I  Forcing finiteness test` warning that can be printed
by GAP if one of the groups is not (yet) known to be finite.
    
URL: sagemath#38956
Reported by: Michael Orlitzky
Reviewer(s): David Coudert
@vbraun vbraun merged commit 4434cfd into sagemath:develop Dec 8, 2024
@user202729
Copy link
Copy Markdown
Contributor

Has been a while, but I wonder if hiding the warning is the best idea. After all, warnings are displayed for a reason.

The original motivation is just to make the doctests pass. For that Sage has a custom SageOutputChecker, where you can implement do_fixup().

@orlitzky
Copy link
Copy Markdown
Contributor Author

Has been a while, but I wonder if hiding the warning is the best idea. After all, warnings are displayed for a reason.

The isomorphism check only works on finite groups, so my thought process was, if it has to check that the groups are finite... okay? Is there any action I should take? If not, it's just noise. Printing unstructured GAP messages to the console also isn't great from the sage UI perspective since there's no easy way to hide them in your own program like there would be with e.g. a python warning.

For power users and developers, the warnings are still available if you really want to see them: just use G.gap().IsomorphismGroups(H.gap()) instead of the sage method.

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

Labels

p: CI fix merged before running CI tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Random test failures in finitely_presented_named

5 participants