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

Disable polynomial ring caching in projective_space and affine_space methods #4094

Merged

Conversation

fingolfin
Copy link
Member

Another part of PR #3865 (and this part of it was already discussed extensively there)

I hope it'll either just pass fine, or if it has problems, then at least just a subset of those in PR #3865.

@simonbrandhorst simonbrandhorst enabled auto-merge (squash) September 16, 2024 09:56
Copy link
Collaborator

@afkafkafk13 afkafkafk13 left a comment

Choose a reason for hiding this comment

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

Looks fine.

@fingolfin fingolfin force-pushed the mh/no-caching-in-ag-constructors branch from 6974109 to 833e4be Compare September 16, 2024 12:03
# ring around the rosie once
@test pX == X(rational_point_coordinates(defining_ideal(scheme(pX))))
Oscar.closed_embedding(pA)
Oscar.closed_embedding(pX)
@test dim(tangent_space(pX))==1
@test dim(tangent_space(pA))==2

A2a = affine_space(GF(2), [:x, :y]);
@test A2a([1,0]) == pA
Copy link
Member Author

Choose a reason for hiding this comment

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

This test failed because now A2 and A2a are not using the same polynomial ring anymore (note that A2 != A2a holds now and also before)

@lgoettgens lgoettgens changed the title Disable polynomial ring caching in projective_space and affine_space methods Disable caching in projective_space and affine_space methods Sep 16, 2024
@lgoettgens lgoettgens changed the title Disable caching in projective_space and affine_space methods Disable polynomial ring caching in projective_space and affine_space methods Sep 16, 2024
@afkafkafk13 afkafkafk13 enabled auto-merge (squash) September 16, 2024 12:43
Copy link

codecov bot commented Sep 16, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.63%. Comparing base (167eb43) to head (11ae75d).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...etry/Schemes/AffineSchemes/Objects/Constructors.jl 75.00% 1 Missing ⚠️
.../Schemes/ProjectiveVariety/Objects/Constructors.jl 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4094      +/-   ##
==========================================
- Coverage   84.63%   84.63%   -0.01%     
==========================================
  Files         614      614              
  Lines       83366    83366              
==========================================
- Hits        70558    70556       -2     
- Misses      12808    12810       +2     
Files with missing lines Coverage Δ
.../Schemes/ProjectiveSchemes/Objects/Constructors.jl 93.44% <100.00%> (ø)
...etry/Schemes/AffineSchemes/Objects/Constructors.jl 75.34% <75.00%> (ø)
.../Schemes/ProjectiveVariety/Objects/Constructors.jl 78.26% <50.00%> (ø)

... and 1 file with indirect coverage changes

@afkafkafk13 afkafkafk13 merged commit c6ceed4 into oscar-system:master Sep 16, 2024
28 checks passed
@fingolfin fingolfin deleted the mh/no-caching-in-ag-constructors branch September 16, 2024 16:36
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