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

Catelnuovo-Mumford regularity #2666

Merged
merged 4 commits into from
Aug 14, 2023
Merged

Catelnuovo-Mumford regularity #2666

merged 4 commits into from
Aug 14, 2023

Conversation

wdecker
Copy link
Collaborator

@wdecker wdecker commented Aug 12, 2023

No description provided.

Copy link
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

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

Some minor remarks

julia> regularity(M)
3

julia> minimal_betti_table(M);
Copy link
Member

Choose a reason for hiding this comment

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

Why would you add the betti table to the doctest, if it doesn't get shown?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a problem with the documenter in printing betti tables. The ; will be removed as soon as this problem is fixed.

src/Modules/UngradedModules.jl Outdated Show resolved Hide resolved
src/Modules/UngradedModules.jl Outdated Show resolved Hide resolved
julia> regularity(I)
6

julia> minimal_betti_table(I);
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as above

src/Rings/mpoly-ideals.jl Outdated Show resolved Hide resolved
@micjoswig
Copy link
Member

Our decision against submodules in OSCAR requires long names throughout. Please rename regularity to CM_regularity or similar.

@codecov
Copy link

codecov bot commented Aug 13, 2023

Codecov Report

Merging #2666 (594a107) into master (47432f8) will decrease coverage by 0.07%.
The diff coverage is 30.55%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2666      +/-   ##
==========================================
- Coverage   72.25%   72.19%   -0.07%     
==========================================
  Files         437      437              
  Lines       61995    62012      +17     
==========================================
- Hits        44796    44769      -27     
- Misses      17199    17243      +44     
Files Changed Coverage Δ
src/Modules/ModulesGraded.jl 61.79% <0.00%> (-0.91%) ⬇️
src/Modules/UngradedModules.jl 76.50% <ø> (ø)
src/Rings/mpoly-ideals.jl 86.43% <ø> (+1.51%) ⬆️
src/Rings/mpoly-graded.jl 79.68% <44.00%> (-1.03%) ⬇️

... and 3 files with indirect coverage changes

@fieker fieker merged commit 3704249 into master Aug 14, 2023
12 of 15 checks passed
@fieker fieker deleted the Wolfram branch August 14, 2023 08:25
HechtiDerLachs pushed a commit to HechtiDerLachs/Oscar.jl that referenced this pull request Aug 17, 2023
* Catelnuovo-Mumford regularity

* React to code review

* correcting once more

* Reacting to @micjoswig
justus-springer pushed a commit to justus-springer/Oscar.jl that referenced this pull request Aug 18, 2023
* Catelnuovo-Mumford regularity

* React to code review

* correcting once more

* Reacting to @micjoswig
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