Varieties v0.3 #3517
Varieties v0.3 #3517
Conversation
|
I'm not sure if @mikestillman has forgotten about mikestillman#38, but this PR also includes those changes. |
|
@d-torrance any idea what's happening here? I don't think this PR changes anything related to this ... --*- compilation -*-
i1 : input "../../../../../Macaulay2/tests/normal/alarm.m2"
ii2 : -- no-check-flag (see https://github.com/Macaulay2/M2/issues/1392)
-- test that alarms work
time try ( alarm(1); while true do 1 ) else true
-- used 1.13874s (cpu); 0.999088s (thread); 0s (gc)
oo2 = true
ii3 :
R = QQ[vars(0..24)]
oo3 = R
oo3 : PolynomialRing
ii4 : f = () -> (alarm 4; try res coker vars R else "ran out of time")
oo4 = f
oo4 : FunctionClosure
ii5 : time f()
*** out of memory trying to allocate 128 bytes, exiting ***
Command exited with non-zero status 1
16.95user 4.55system 0:11.87elapsed 181%CPU (0avgtext+0avgdata 477640maxresident)k
0inputs+8outputs (0major+13659minor)pagefaults 0swaps |
|
Not sure. Apparently this test has been a problem before since we actually skip it when when running |
|
@mahrud - Do you think it's worth applying the |
|
Sure -- it looks like this PR is on top of that one, so either way. :) Sounds good! |
| drop(ws := separate_" " s, -1), | ||
| if plurals#?(last ws) | ||
| then plurals#(last ws) else last ws | "s") | ||
| pluralsynonym = T -> if T.?synonym then pluralize T.synonym else "objects of class " | toString T |
There was a problem hiding this comment.
Why are these changes part of a pull request focusing on Varieties? I would have guessed that they were entirely unrelated.
There was a problem hiding this comment.
It's included to make the following error print correctly:
i3 : directSum(Proj QQ[x], Proj QQ[y])
stdio:2:9:(3): error: no method for direct sum of projective varieties
i5 : directSum(Spec QQ[x], Spec QQ[y])
stdio:4:9:(3): error: no method for direct sum of affine varieties
| inducedMap(P, T) * map(T, target g, T_[1], Degree => - degree g)}; | ||
| P.cache.formation = FunctionApplication (pushout, args); | ||
| P.cache.pushoutMaps = apply(#args, | ||
| i -> inducedMap(P, T) * map(T, target args#i, T_[i], Degree => - degree args#i)); |
There was a problem hiding this comment.
Are the pullback and pushout documented or tested anywhere?
There was a problem hiding this comment.
They are tested in Varieties, but the documentation is only a stump.
| @@ -64,7 +64,7 @@ placeholderToSchubertBasis(RingElement,FlagBundle) := (c,G) -> ( | |||
| (k,q) := toSequence(G.BundleRanks); | |||
| P := diagrams(q,k); | |||
| M := apply(P, i-> placeholderSchubertCycle(i,G)); | |||
| E := flatten entries basis(R); | |||
| E := flatten entries basis(R, Variables => 0 .. numgens R - 1); | |||
There was a problem hiding this comment.
This appears to depend on other changes to basis, which I am personally not yet sold on.
M2/Macaulay2/packages/DGAlgebras.m2
Outdated
| @@ -203,7 +203,7 @@ getBasis (ZZ,DGAlgebra) := opts -> (homDegree,A) -> getBasis(homDegree,A.natural | |||
| getBasis (ZZ,Ring) := opts -> (homDegree,R) -> ( | |||
| local retVal; | |||
| myMap := map(R, R.cache.basisAlgebra); | |||
| tempList := (flatten entries basis(homDegree, R.cache.basisAlgebra, Limit => opts.Limit)) / myMap; | |||
| tempList := (flatten entries basis(homDegree, R.cache.basisAlgebra, Limit => opts.Limit, Variables => 0 .. numgens R-1)) / myMap; | |||
There was a problem hiding this comment.
This seems to depend on other changes to basis?
| -- TODO: should matrix RingMap return this map instead? | ||
| -- mon := map(module T, image matrix(S, { S.FlatMonoid_* }), f, matrix f) | ||
| mat := last coefficients(f cover srcgens, Monomials => targens); | ||
| map(image targens, image srcgens, f, mat)) |
There was a problem hiding this comment.
Aren't these many changes to basis already part of an independent pull request?
There was a problem hiding this comment.
Yes, but this PR is on top of that because of actual dependencies (but it is not on top of the part PR).
| @@ -177,7 +177,7 @@ nefGenerators NormalToricVariety := Matrix => X -> ( | |||
| fromPic := matrix fromPicToCl X; | |||
| indexOfPic := abs lcm (minors( rank source fromPic, fromPic^torsionlessCoord))_*; | |||
| (indexOfPic * coneGens) // fromPic | |||
| ); | |||
| )) | |||
There was a problem hiding this comment.
By definition, the nef cone lives in the Neron--Severi group (which for a toric variety is canonical isomorphic to the Picard group). Does returning something in the class group make mathematical sense?
There was a problem hiding this comment.
Sorry, I don't follow. The only change here is caching of nefGenerators. What change are you referring to?
It is decoupled. I don't remember exactly why, but the changes to basis stemmed from fixing things in Varieties, which is why this PR is on top of that one.
I tried to do this a while ago but I'm afraid it was very complicated (random tests kept failing in one package or another when I tried to separate them) and I gave up, so I don't want to do it. If you or someone else wants to give it an attempt you're welcome to. |
mahrud
force-pushed
the
SheafMaps
branch
2 times, most recently
from
c6eead8
to
be00565
Compare
|
Okay, updates:
|
There was a problem hiding this comment.
Once @ggsmith is happy with this, it looks fine to me, with the caveat that I'm not so convinced that the breaking change to basis is worth the breaking. I think basis needs a complete revamping at some point, with more expressive names for what it is doing. But other than that, this all looks good. I did not go into all the Varieties code in detail though. I am happy to see complexes of sheaves though!
|
Argh this isn't merged, I just made a mistake. I'll reopen a new one. |
This PR contains work throughout Spring and Summer 2024, including at the workshop at Utah. Complexes of sheaves are added, but are currently disabled until the Complexes package can be preloaded. Lots of work remains.
cc: @kellerlv @ggsmith @Devlin-Mallory @ritvikrr @johndcobb