-
Notifications
You must be signed in to change notification settings - Fork 240
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
added package GKMVarieties #1505
Conversation
HomePage => "https://github.com/chrisweur/GKMManifolds", | ||
PackageExports => {"Graphs", "Matroids", "NormalToricVarieties"}, | ||
AuxiliaryFiles => true, | ||
DebuggingMode => true |
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.
please turn off debugging mode (that's only for developers of the package)
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.
done
--G.HTpt : a polynomial ring, represent the T-equivariant cohomology ring of a point | ||
|
||
|
||
MomentGraph = new Type of MutableHashTable |
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.
Since you have a cache table, maybe this doesn't have to be mutable. Then you could compare two moment graphs with "===".
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.
MomentGraph is a HashTable now.
tVariety(MomentGraph,Ring) := TVariety => (G,R) -> ( | ||
if not #(gens R) == #(gens G.HTpt) then ( | ||
<< "HTpt not compatible with the character ring" << | ||
return error |
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.
Probably you want to replace these two lines by error "HTpt not compatible with the character ring"
.
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.
done
--returns the moment graph of the T-variety X if it is defined | ||
momentGraph(TVariety) := MomentGraph => X -> ( | ||
if X.?momentGraph then X.momentGraph | ||
else << "no moment graph defined for this T-variety" << return error |
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.
Similarly here.
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.
done
print " warning: overwriting a previously defined moment graph on this T-variety " | ||
); | ||
if #(gens X.charRing) != #(gens G.HTpt) then ( | ||
<< "HTpt not compatible with the character ring" << |
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.
... and here and lots of places below.
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.
done
@@ -0,0 +1,2109 @@ | |||
|
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.
"Documentation" is already a collective noun and should not be made plural (in the name of this file)
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.
yes
The tests are failing due to invalid html: -- validating all html and xhtml files in /home/runner/work/M2/M2/M2/BUILD/build/usr-dist/common/share/doc/Macaulay2
*** invalid HTML: /home/runner/work/M2/M2/M2/BUILD/build/usr-dist/common/share/doc/Macaulay2/GKMManifolds/html/___Example_co_spgeneralized_spflag_spvarieties.html
error: line 34: document type does not allow element "table" here; missing one of "button", "map", "object", "ins", "del", "noscript" start-tag
error: line 40: document type does not allow element "table" here; missing one of "button", "map", "object", "ins", "del", "noscript" start-tag
error: line 46: document type does not allow element "table" here; missing one of "button", "map", "object", "ins", "del", "noscript" start-tag
8561 HTML files checked; 1 invalid
make: *** [validate-html] Error 1
GNUmakefile:308: recipe for target 'validate-html' failed
make: Leaving directory '/home/runner/work/M2/M2/M2/BUILD/build'
##[error]Process completed with exit code 2. |
Thank you for the submission -- it looks like you've put a lot of work into it! |
I'm not sure what's happening with the test failing ("due to invalid html"). Everything runs fine locally on my computer. Is there something I need to do? |
@chrisweur This is caused by the outdated way in which M2 converts latex matrices to html tables instead of rendering the latex, compounded with silly xhtml rules that forbid tables inside paragraphs. For now, use two dollar signs instead of one dollar sign around diff --git a/M2/Macaulay2/packages/GKMManifolds/Documentation_GKMManifolds.m2 b/M2/Macaulay2/packages/GKMManifolds/Documentation_GKMManifolds.m2
index 34d5697e8..3624c93c6 100644
--- a/M2/Macaulay2/packages/GKMManifolds/Documentation_GKMManifolds.m2
+++ b/M2/Macaulay2/packages/GKMManifolds/Documentation_GKMManifolds.m2
@@ -78,16 +78,16 @@ doc ///
Text
For type $B_n$, the group $G$ is $SO_{2n+1}$, where we set the standard symmetric bilinear form on $\mathbb C^{2n+1}$
- to be is given by the matrix $\begin{pmatrix} 0 & I_n & 0 \\ I_n & 0 & 0 \\ 0 & 0 & 1 \end{pmatrix}$ and the torus $T$ is $diag(t_1, \ldots,t_n, t_1^{-1}, \ldots, t_n^{-1}, 1)$.
+ to be is given by the matrix $$\begin{pmatrix} 0 & I_n & 0 \\ I_n & 0 & 0 \\ 0 & 0 & 1 \end{pmatrix}$$ and the torus $T$ is $diag(t_1, \ldots,t_n, t_1^{-1}, \ldots, t_n^{-1}, 1)$.
Text
For type $C_n$, the group $G$ is $Sp_{2n}$, where we set the standard alternating bilinear form on
- $\mathbb C^{2n}$ to be given by the matrix $\begin{pmatrix} 0 & -I_n \\ I_n & 0 \end{pmatrix}$
+ $\mathbb C^{2n}$ to be given by the matrix $$\begin{pmatrix} 0 & -I_n \\ I_n & 0 \end{pmatrix}$$
and the torus $T$ is $diag(t_1, \ldots,t_n, t_1^{-1}, \ldots, t_n^{-1})$.
Text
For type $D_n$, the group $G$ is $SO_{2n}$, where we set the standard symmetric bilinear form on
- $\mathbb C^{2n}$ to be given by the matrix $\begin{pmatrix} 0 & I_n \\ I_n & 0 \end{pmatrix}$
+ $\mathbb C^{2n}$ to be given by the matrix $$\begin{pmatrix} 0 & I_n \\ I_n & 0 \end{pmatrix}$$
and the torus $T$ is $diag(t_1, \ldots,t_n, t_1^{-1} \ldots, t_n^{-1})$.
Text @DanGrayson, merging #1475 would avoid this issue, and also produce an objectively better result while remaining compliant with xhtml rules. Here is a comparison: |
@mahrud : thanks for the suggestions; I have just implemented them. As for the keyword, I thought "Equivariant Cohomology" could be more appropriate. The m2 file has the line Keyword => {"Equivariant Cohomology", ... } now, but commented out. |
--"tHilbNumer", | ||
"RREFMethod", | ||
"latticePts" | ||
} |
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.
Can you come up with less cryptic names for these identifiers? The especially cryptic ones are "HTpt", "tvar", "hilb", "tMap", "TKclass", etc. Even "latticePts" could be expanded to "latticePoints".
By the way, if "T" in some of the identifiers refers to your favorite symbol for a torus, then what about users whose favorite name is "S"?
error " the first entry must be one of \"A\", \"B\", \"C\", or \"D\" " | ||
); | ||
if d <= 0 then ( | ||
error " the second entry (dimension) must be a positive integer " |
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.
same here
tGeneralizedFlagVariety = method() | ||
tGeneralizedFlagVariety(String,ZZ,List) := TVariety => (LT,d,L) -> ( | ||
if not member(LT,{"A","B","C","D"}) then ( | ||
error " the first entry must be one of \"A\", \"B\", \"C\", or \"D\" " |
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 mean "argument", not "entry".
apply(delete(v,extrWts), w -> ( | ||
lambda := toRootFunc(w-v); | ||
if #lambda == 1 then ( | ||
lambdas = lambdas | lambda; |
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.
repeatedly lengthening a list can make an algorithm order N^2 instead of order N. Could you just collect the results in a list and then concatenate them all at the end?
|
||
---------------------------< auxiliary functions for TOrbClosure >-------------------------------- | ||
convertToNum = (n,L) -> ( | ||
return apply(toList L, v -> if v === unastrsk(v) then v else n + unastrsk v) |
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.
"return" is redundant here
|
||
doc /// | ||
Key | ||
tMap |
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.
It could be something like "makeGKMMap".
pushforward | ||
(pushforward, TMap) | ||
Headline | ||
computes the pushforward map of T-equivariant K-classes of a T-map |
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.
starting so many headlines with "computes the" seems redundant, since every function computes something.
doc /// | ||
Key | ||
tChi | ||
(tChi, TKClass) |
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.
cryptic. What about "eulerCharacteristic"?
|
||
doc /// | ||
Key | ||
kTutte |
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.
What does "k" signify here? It's pretty cryptic.
|
||
doc /// | ||
Key | ||
setIndicator |
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.
Simpler might be "indicator".
@DanGrayson : thanks for a careful review and many suggestions. For the naming of of methods/identifiers, we agree some need to be more suggestive. As for the naming of many things with "T"s, we are having some trouble, and would appreciate what your thoughts are after looking at the following explanation behind our choices. We had given the use of "T" quite some thought, but (unfortunately) ended up with what we have now due to several concerns with other potential names. For example: (1) We considered "torusVariety" and "torusMap" for "TVariety" and "TMap", but "torusVariety" seemed to wrongly indicate either a "toric variety" or worse an "an algebraic torus considered as a variety", and moreover, a "torusMap" really sounded like a map between tori rather than a torus-equivariant map between two varieties. We could have done "EquivariantMap", but putting "equivariant" everywhere seemed to make all the names way too long and clunky; for example, "makeEquivariantKClass" seemed a bit too long. (2) We also considered "GKMManifold" and "GKMMap", but a "GKMMap" in particular seemed misleading because it really is just a torus-equivariant map---there is no "GKM"-ness in the map itself. Same issue (but worse) persists with, for example, "GKMKClass" or "GKMEulerCharacteristic". Eventually, we decided to take up the wide-spread convention that varieties with an action of a group G are denoted G-varieties, and equivariant maps between them G-maps. The letter "T" may not be the favorite notation for an algebraic torus for everyone, but in all the references mentioned in the documentation (with the exception of two where the group is more general than an algebraic torus), and in almost all references concerning torus-equivariant Chow classes (not implemented currently, but hopefully in the future), an algebraic torus had the letter "T". To us, the letter "T" seemed universal enough so as to not making too many people unhappy. Thus, despite the shortcomings, we ended up deciding to take up the convention that an already established object [name] (like "Variety, Map, etc.") will have a counterpart in this package with the name "T[name]". We recognize that this may not be ideal, and if you have suggestions we are eager to consider them. |
Normally one doesn't call something a T-variety until after one has introduced a group variety and named it T. One may also have groups called, G and H, and hence have G-varieties and H-varieties. Maybe we could take standard mathematical tradition that to heart and introduce a function that accepts a group variety and produces a type. (We didn't have to do that for "R-module", because "module" is a word without other applications, so we named the type "Module".) It won't be hard to code -- the problem is selecting the names and the syntax. What if we try to make Then what would be the type of equivariant maps between two such things? In general, it seems to me that we can make the type of maps between two objects X and Y of type |
@DanGrayson: Thanks; this certainly could be a good solution, assuming that implementing it won't be too difficult. Are there any examples, say in other packages or in M2 core, that I can look at? If it looks like it can be done in few (2~4) full days, I can certainly make these changes. |
I can't think of any good existing examples. Here's the way it might go: --code
exportFrom(Core, {"getAttribute","hasAttribute","ReverseDictionary"})
Torus = new Type of HashTable
globalAssignment Torus
net Torus := toString Torus := T -> (
if hasAttribute(T,ReverseDictionary)
then toString getAttribute(T,ReverseDictionary)
else (
-- temporary
if T#?"underlying variety"
then "<< a torus with underlying variety "|toString T#"underlying variety"|" >>"
else "<< a torus >>"
)
);
VarietyWithTorusActing = new Type of Type
globalAssignment VarietyWithTorusActing
Torus - Type := (T,Variety') -> (
if Variety' =!= Variety then error "group types implemented only for the type 'Variety'";
TVar := new VarietyWithTorusActing of HashTable;
TVar # "the torus" = T;
net TVar := toString TVar := X -> (
if hasAttribute(X,ReverseDictionary)
then toString getAttribute(X,ReverseDictionary)
else "<< a variety "|toString X#"underlying variety"|" acted upon by "|toString T|" >>" -- temporary
);
TVar
)
net VarietyWithTorusActing := TVar -> toString TVar#"the torus" | "-Variety";
--demo
T := new Torus from {
"underlying variety" => Spec (QQ[t,Inverses=>true,MonomialOrder => Lex])
}
S = T
R = QQ[x,y,z]
X = Proj R
S-Variety
Y = new S-Variety from { "underlying variety" => X } |
Here's the demo part of the output from that: ii10 : --demo
T := new Torus from {
"underlying variety" => Spec (QQ[t,Inverses=>true,MonomialOrder => Lex])
}
oo10 = << a torus with underlying variety Spec(QQ[t]) >>
oo10 : Torus
ii11 : S = T
oo11 = S
oo11 : Torus
ii12 : R = QQ[x,y,z]
oo12 = R
oo12 : PolynomialRing
ii13 : X = Proj R
oo13 = X
oo13 : ProjectiveVariety
ii14 : S-Variety
oo14 = S-Variety
oo14 : VarietyWithTorusActing
ii15 : Y = new S-Variety from { "underlying variety" => X }
oo15 = << a variety X acted upon by S >>
oo15 : S-Variety |
Let me know when your changes are done, for now. |
All recommended changes are done, for now. I'm not sure why one of the build test is failing... |
Ignore the failing ThreadedGB example. It's not related. |
Thank you for the contribution! |
GKM Manifolds are certain (smooth, complete) varieties with an action of a torus for which many equivariant cohomology and K-theory computations can be carried out combinatorially via their moment graphs. Ritvik Ramkumar and I have developed a M2 package that we believe is now mature enough to be included in the distribution.