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

Repair quotient rings #2788

Closed

Conversation

HechtiDerLachs
Copy link
Collaborator

@thofma
Copy link
Collaborator

thofma commented Sep 12, 2023

Was it decided at one of the Kaiserslautern meetings, that we'd rather have a hash functions which errors than a slow default one?

@HechtiDerLachs
Copy link
Collaborator Author

Yes. This was @fieker 's proposal.

@lgoettgens
Copy link
Member

Was it decided at one of the Kaiserslautern meetings, that we'd rather have a hash functions which errors than a slow default one?

This is similar to oscar-system/GAP.jl#891.

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #2788 (4e0bf93) into master (79874fd) will decrease coverage by 0.05%.
Report is 5 commits behind head on master.
The diff coverage is 73.04%.

@@            Coverage Diff             @@
##           master    #2788      +/-   ##
==========================================
- Coverage   73.63%   73.58%   -0.05%     
==========================================
  Files         455      455              
  Lines       64411    64443      +32     
==========================================
- Hits        47427    47422       -5     
- Misses      16984    17021      +37     
Files Changed Coverage
experimental/JuLie/src/schur_polynomials.jl ø
...ntal/QuadFormAndIsom/src/lattices_with_isometry.jl ø
experimental/Schemes/CoveredProjectiveSchemes.jl ø
experimental/Schemes/elliptic_surface.jl ø
src/AlgebraicGeometry/Schemes/Glueing/Methods.jl ø
...eometry/Schemes/ProjectiveSchemes/Objects/Types.jl ø
src/Modules/FreeModules-graded.jl 0.00%
src/Modules/ModulesGraded.jl 0.00%
src/Rings/mpoly-affine-algebras.jl 0.00%
src/Rings/mpoly-graded.jl 0.00%
... and 14 more

@HechtiDerLachs
Copy link
Collaborator Author

Looks like the code coverage report is nothing that can be taken serious these days. All coverage changes occur in places that I did not touch; not even indirectly! I suppose this is due to the various failing tests?

Either way: I would like to point to #2789 if anyone feels like doing a proper review.

@HechtiDerLachs HechtiDerLachs marked this pull request as draft September 12, 2023 15:00
Comment on lines +851 to +853
f.f == g.f && return true
hash(f) == hash(g) && return true # calls simplify already
return f.f == g.f
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks wrong to me. Please double check

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. What was I thinking...? Corrected in #2789 .

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.

4 participants