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

Fix incorrect IsFinitelyGeneratedMonoid method #2280

Merged

Conversation

fingolfin
Copy link
Member

Turns out we also have IsFinitelyGeneratedMonoid (which I did not realize), and we also have an invalid method for that (see PR #2275).

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them topic: library labels Mar 22, 2018
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.

Thanks for this fix.

(In an ideal world, there would be no properties IsFinitelyGeneratedMonoid and IsFinitelyGeneratedGroup, see the discussion about #2252.)

A finite generating set implies finite generation; but an infinite
generating set does not mean that there cannot be a finite one, too.
@fingolfin fingolfin force-pushed the mh/fix-IsFinitelyGeneratedMonoid branch from 3ebffe0 to 8a338d7 Compare March 22, 2018 15:11
@codecov
Copy link

codecov bot commented Mar 22, 2018

Codecov Report

Merging #2280 into master will decrease coverage by <.01%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #2280      +/-   ##
==========================================
- Coverage   71.36%   71.36%   -0.01%     
==========================================
  Files         480      480              
  Lines      253198   253202       +4     
==========================================
+ Hits       180697   180698       +1     
- Misses      72501    72504       +3
Impacted Files Coverage Δ
lib/monofree.gi 80.99% <100%> (+0.15%) ⬆️
lib/monoid.gi 53.38% <75%> (+0.3%) ⬆️
src/hpc/threadapi.c 36.71% <0%> (-0.19%) ⬇️
src/io.c 61.33% <0%> (-0.09%) ⬇️
hpcgap/lib/hpc/stdtasks.g 38.87% <0%> (+0.25%) ⬆️

@fingolfin
Copy link
Member Author

Yes, I agree on the "ideal world" remark, and I hope we can get there -- I just thought that in the meantime, it'd be a good idea to fix the bugs immediately as I notice them

@fingolfin
Copy link
Member Author

There was a test which required infinitely generated free monoids to know have IsFinitelyGeneratedMonoid(M) = false, which caused a test failure; this should now be resolved.

@fingolfin fingolfin merged commit ac64e83 into gap-system:master Mar 23, 2018
@fingolfin fingolfin deleted the mh/fix-IsFinitelyGeneratedMonoid branch March 23, 2018 14:56
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants