-
Notifications
You must be signed in to change notification settings - Fork 126
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
Some serialization of schemes #4110
base: master
Are you sure you want to change the base?
Some serialization of schemes #4110
Conversation
338cfa7
to
7ac8e7d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4110 +/- ##
==========================================
+ Coverage 84.67% 84.71% +0.03%
==========================================
Files 626 627 +1
Lines 83980 84176 +196
==========================================
+ Hits 71108 71306 +198
+ Misses 12872 12870 -2
|
@check begin | ||
d = prod(length(f) > 0 ? f : one(OO(X))) | ||
U = spec(R) | ||
U == hypersurface_complement(X, d) | ||
end | ||
return new{base_ring_type(X), ring_type(U), typeof(X)}(X, U, lifted_numerator.(f)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks fishy to me. U
is not defined if check=false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I'll look into this.
1a9a316
to
5e4398d
Compare
test/Serialization/IPC.jl
Outdated
@testset "parallel smoothness test for schemes" begin | ||
channels = Oscar.params_channels(Union{AffineScheme, Ring}) | ||
|
||
IP2 = projective_space(QQ, [:x, :y, :z]) | ||
S = homogeneous_coordinate_ring(IP2) | ||
(x, y, z) = gens(S) | ||
|
||
I = ideal(S, y^2*z + x^3 + x^2*z) | ||
|
||
X, _ = sub(IP2, I) | ||
X_cov = covered_scheme(X) | ||
U = affine_charts(X_cov) | ||
for a in U | ||
Oscar.put_params(channels, ambient_coordinate_ring(a)) | ||
Oscar.put_params(channels, OO(a)) | ||
end | ||
results = pmap(is_smooth, U) | ||
@test results == [1, 1, 0] | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @simonbrandhorst , @afkafkafk13 .
Can we merge #4121 first and then get a better view on what's going on here? It seems that this is built on top of the other. |
bc12d64
to
6ccadd7
Compare
@antonydellavecchia : I did another implementation which broadcasts all rings to all workers using |
@testset "_is_smooth" begin | ||
R, (x, y, z) = QQ[:x, :y, :z] | ||
|
||
I1 = ideal(R, x*y) | ||
IA3 = spec(R) | ||
X1, _ = sub(IA3, I1) | ||
@test !Oscar._is_smooth(X1) | ||
@test !Oscar._is_smooth(X1; jacobian_cut_off=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@afkafkafk13 : I think I have now implemented a truly generic smoothness test here which can also handle non-reduced, non-equidimensional, and non-connected cases. I'm still a bit skeptic, since all the classical implementations seem to solve this via some primary/equidimensional decomposition which I seem to be able to fully avoid. If you have the time: Could you have a look and tell me what you think? Maybe you have some further exotic example in mind for which my implementation fails. If so, it would be good to know about it.
ping @danteluber : Check out this PR and try the following. using Distributed
process_ids = addprocs(4)
@everywhere using Oscar That starts the workers. Then you can do
Replace |
Intermediate state of the art of our joint effort with @antonydellavecchia this week.