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

Introduce -inf as potential output for the dimension of an ideal #4393

Merged
merged 15 commits into from
Dec 11, 2024

Conversation

HechtiDerLachs
Copy link
Collaborator

Address #2721 .

@HechtiDerLachs
Copy link
Collaborator Author

Tests seem to pass. Good to go from my side.

@HechtiDerLachs HechtiDerLachs changed the title dimension of an ideal can be -inf Introduce -inf as potential output for the dimension of an ideal Dec 11, 2024
function dim(I::MPolyIdeal)
if I.dim === nothing
if is_zero(ngens(base_ring(I))) # Catch a boundary case
I.dim = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, is this correct if the coefficient ring is not a field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess you're right. It was probably wrong before, but we should correct it.

Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 92.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.40%. Comparing base (d6230ef) to head (67ae9ac).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
experimental/Schemes/src/Resolution_structure.jl 83.33% 1 Missing ⚠️
...ebraicGeometry/Schemes/Divisors/AlgebraicCycles.jl 0.00% 1 Missing ⚠️
src/Rings/mpoly-ideals.jl 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4393      +/-   ##
==========================================
+ Coverage   84.38%   84.40%   +0.01%     
==========================================
  Files         656      656              
  Lines       87198    87216      +18     
==========================================
+ Hits        73583    73613      +30     
+ Misses      13615    13603      -12     
Files with missing lines Coverage Δ
...y/Schemes/AffineAlgebraicSet/Objects/Properties.jl 100.00% <100.00%> (ø)
...ometry/Schemes/AffineSchemes/Objects/Attributes.jl 91.72% <100.00%> (ø)
...metry/Schemes/CoveredSchemes/Objects/Attributes.jl 85.93% <100.00%> (ø)
...ry/Schemes/ProjectiveSchemes/Objects/Attributes.jl 87.75% <100.00%> (ø)
.../AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl 81.53% <100.00%> (ø)
src/Rings/MPolyQuo.jl 92.08% <100.00%> (ø)
src/Rings/mpoly-affine-algebras.jl 82.58% <100.00%> (ø)
src/Rings/mpoly-localizations.jl 77.82% <100.00%> (+0.16%) ⬆️
src/Rings/mpoly.jl 72.23% <100.00%> (ø)
src/Rings/mpolyquo-localizations.jl 74.46% <100.00%> (+0.01%) ⬆️
... and 3 more

... and 1 file with indirect coverage changes

@simonbrandhorst simonbrandhorst merged commit 23b0b9d into oscar-system:master Dec 11, 2024
28 of 30 checks passed
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.

3 participants