Add toric Chow group

[vbraun]

The toric Chow group is a useful version of a homology theory for toric varieties, especially for singular varieties. The attached patch provides support for computing the toric Chow group as well as elementary intersection theory.

See tracker bug at #9604 to for related patches.

Apply:

1. attachment: trac_9713_fix_cardinality.patch
2. attachment: trac_9713_fix_fg_pid.2.patch
3. attachment: trac_9713_fix_fg_pid_reviewer.2.patch
4. attachment: trac_9713_toric_chow_group.patch
5. attachment: 9713_hash.patch

Depends on #8948.

CC: @novoselt

Component: algebraic geometry

Author: Volker Braun

Reviewer: Andrey Novoseltsev, Simon King, Jeroen Demeyer

Merged: sage-4.6.2.alpha4

Issue created by migration from https://trac.sagemath.org/ticket/9713

[novoselt]

comment:1

Volker, is this one ready for review? (It needs a little rebasement on top of new #9337.)

[vbraun]

comment:2

I think its ready, but you will probably soon find something to disagree on ;-)

Rebased patch is attached.

[vbraun]

Reviewer: Andrey Novoseltsev

[novoselt]

comment:3

Some preliminary comments:

1. The new module is not completely documented yet, sage -coverage shows some issues.
2. It seems to me that it is more common to use A^k rather than A_d-k, so I think it would be better to stick with it in the documentation.
3. I'd prefer to drop "The " from "The Chow cycle ..." in _repr_ as was done for divisor classes.
4. It is not clear how to construct Chow cycles (except for those that correspond to cones).
5. It is not clear what do numbers mean in the representation - i.e. how the basis is chosen?
6. Regardless of the chosen basis, I think it would be more useful to see cycles as linear combination of cones (perhaps with cones represented by their ambient_ray_indices), without seeing those that got zero coefficients, of course. This issue is also relevant for divisor/cohomology classes and for divisor classes we do use coordinates in some basis, but I think that it makes more sense there, while here basis elements can be given by cycles of different dimension and having no separation between them makes it difficult to interpret the output.

[vbraun]

comment:4

I'll try to add the missing docstrings.

I was intentionally using A_k and not A^{d-k} because this patch implements the Chow homology and not the cohomology. I'm not sure to which extend they are the same over singular/noncompact varieties over ZZ.

I'm not entirely happy with the output either. But printing all the cones (even if we abbreviate them) will very quickly produce multi-line output. Right now, the output is essentially in a random basis corresponding to cycle.parent().degree().

sage: cycle = X.Chow_group().gen(1)
sage: cycle
The Chow cycle (0, 1, 0, 0, 0, 0)
sage: cycle.parent().degree()
(Z, Z x Z, Z x Z, Z)
sage: cycle.lift()
(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0)

For all practical purposes, the user should use lift() to get the coefficients of the cones in the given order flatten(toric_variety.fan().cones()). Would printing

Chow cycle ((0,), (1, 0), (0, 0), (0,))

make you more happy? Note that some methods (in particular, intersection with a divisor) need to change the cycle within its Chow class, so even "simple" cycles will produce output where all coefficients are turned on. This is part of why I don't want to print the cone information explicitly.

[novoselt]

comment:5

OK, let's stick to A_k.

Chow cycle ((0,), (1, 0), (0, 0), (0,))

is a bit ugly ;-) Maybe to have it as a vector but separate degrees something like this may work:

Chow cycle (0 | 1, 0 | 0, 0 | 0)

My concern is that if I create the Chow cycle corresponding to a cone generated by rays 2 and 5, it would be nice to see it clearly in the output, say

Chow cycle {2, 5}

with combinations printed as

Chow cycle 2*{2, 5} - 7*{1, 2, 3}

But to have it nicely will require storing the original cone combination, since lift algorithm from a quotient element will involve a lot of cones, as you pointed out. I don't know if it worths the benefits. But it would be nice at least to have some markers between degrees.

[vbraun]

comment:6

I've changed the printing of Chow cycles to the following:

sage: X = toric_varieties.Cube_deformation(7)
sage: A = X.Chow_group()
sage: A.degree()
(Z, C7, Z^5 x C2 x C2, Z)
sage: a = sum( A.gen(i) * (i+1) for i in range(0,A.ngens()) )   # an element of A
sage: a
( 3 | 1 mod 7 | 0 mod 2, 1 mod 2, 4, 5, 6, 7, 8 | 9 )

I'll upload a new patch when #9972 is finished, as this patch will require some rediffing.

[vbraun]

comment:7

Updated patch now applies cleanly. Ready for review...

[novoselt]

comment:8

I am a bit confused by the first example:

sage: A.degree()
(Z, C7, Z^5 x C2 x C2, Z)
sage: a = sum( A.gen(i) * (i+1) for i in range(0,A.ngens()) )   # an element of A
sage: a
( 3 | 1 mod 7 | 0 mod 2, 1 mod 2, 4, 5, 6, 7, 8 | 9 )

with explanation "The Chow group elements are printed as ( a0 | a1 mod 7 | a2 mod 2, a3 mod 2, a4, a5, a6, a7, a8 | a9 ), which denotes the element of the Chow group in the same basis as A.degree()."

A.degree() shows the third component as Z^5 x C2 x C2, yet it seems that elements put cyclic groups first, or maybe it is the reverse order (which would matter for several cyclic groups of different degrees).

[novoselt]

Work Issues: coordinate order of components

[novoselt]

comment:9

A little optional note: it may be better to use \QQ etc. rather than \mathbb{Q}. I personally prefer the look of the last variant, but using standard macros brings more uniformity to the whole Sage documentation.

[vbraun]

comment:10

Done. Now prints as

sage: A.degree()
(Z, C7, C2 x C2 x Z^5, Z)

because thats how FGP_Module organizes its generators (first torsion, then free generators).

[kcrisman]

comment:11

Just a minor point - there is a lot of reference to the Chow group of this and that, but it's apparently only the 'toric' Chow group, as noted at the beginning of the file. Is it typical in the literature to conflate these notions - or is there a theorem that they are equivalent for toric varieties? This should be clarified somehow, maybe even with an example of where they are not the same. It would be unfortunate if they were not the same and someone later wanted to implement regular Chow groups!

Forgive me if the question is naive; I am fairly familiar with 'regular' Chow groups but hadn't encountered this (natural) version before. If there is a foundational paper, it would be helpful to refer to that in the documentation as well; the Wikipedia page referenced doesn't actually refer to the 'toric' variety at all.

[vbraun]

comment:12

The toric Chow cycles are the torus-invariant subvarieties only. Its a standard construction, see e.g. Fultons book. I don't think that there is any hope of it being the same as the full Chow group. But then the Chow group is in general huge and there is little hope of a complete description, and even less of a toric algorithm. I guess we could sprinkle some more "toric_" prefixes around in anticipation of someone inventing the requisite mathematics. But I think its clear enough...

[kcrisman]

comment:13

Replying to @vbraun:

The toric Chow cycles are the torus-invariant subvarieties only.

Yes, I got that.

Its a standard construction, see e.g. Fultons book. I don't think that there is any hope of it being the same as the full Chow group.

Ah, when you say "Fulton's book" I first got out Intersection Theory :) but here it is. Sections 3.3 and 3.4 seem quite relevant. Page 63 says that Pic and the divisor piece of the Chow group can be computed only with what Fulton consistently calls "T-Weil" and "T-Cartier" divisors. Further, the first proposition in Chapter 5 certainly seems to imply that this is in fact true in general for toric varieties. The 'orbit closures' seem to be toric subvarieties by definition, and they generate the Chow group. Am I reading this wrong?

[vbraun]

comment:14

The T-Weil divisor class group is the divisor part of the toric Chow group, and the T-Cartier divisor class group is the Pic (not the other way round). And the orbit closures are the toric subvarieties. As usual, the divisor part of the Chow group is easy; the higher Chow groups become more complicated. Once you understand these, the Hodge conjecture follows easily ;-)

[kcrisman]

comment:15

Yes, I didn't intend to switch those but did by accident. Sorry.

Maybe I wasn't clear. I'm saying that the propositions I reference seem to imply that the regular Chow groups ARE the toric Chow groups, because the free Abelian group on p-cycles turns out to be generated by the toric p-cycles in a toric subvariety. But again, maybe I'm misreading them somehow - are there fewer relations? But the definitions there seem to have the same notion of rational equivalence (?). Unfortunately, I don't have it on me, it was at work I checked this out.

Oh, and don't call them higher Chow groups; that is something else again :) unless you want to start talking about the so-called Beilinson-Hodge conjecture

[novoselt]

Changed work issues from coordinate order of components to none

[novoselt]

comment:16

Here it is, I called the fan Sigma rather than Delta:

Proposition The Chow group Ak(X) of an arbitrary toric variety X = X(Sigma) is generated by the classes of the orbit closures V(sigma) of the cones of dimension n-k of Sigma.

So yes, the patch does implement the "honest Chow group" with the exception that arbitrary subvarieties cannot be translated into their Chow cycles (I think).

Karl, do you plan to review the patch completely? It is fine if no, I am going to do it in the near future. But if yes, it certainly will be great to have more people looking at our toric stuff ;-)

[kcrisman]

comment:17

Yes, I think that is the case - nicely summed up.

No, I am not at all an expert on toric varieties (though I wish I had learned more about them in graduate school now, they are so concrete), so I can't help review. The title of the ticket was just too good not to click on, and then I had this minor point which ballooned into a conversation. Since of course lots of stuff IS toric that one might care about, I am glad you are doing it, but I'm sorry that it's above my pay grade for now. If I had a sabbatical right now we might be talking ;)

[vbraun]

comment:18

By "higher", I mean of course "lower degree" Chow groups. I'm pretty sure its correct in the module documentation :-)

But the free Abelian group of p-cycles is not generated by the toric p-cycles; The former is never finitely generated while the latter always is. In the case of divisors, the relations save you (the Pic^0 is a point). But already for the curves part of the Chow group of a three-dimensional variety its not clear to me that the full Chow group is finitely generated.

[novoselt]

comment:19

Well, Fulton claims that quotients are the same even though the starting free groups are very different ;-)

So I think we should include this proposition in the top of the module level documentation and comment that this allows us to worry only about orbit closures.

[vbraun]

comment:20

The relevant paper is Fulton/MacPherson/Sottile/Sturmfels: Intersection theory on spherical varieties. The bottom line is that, for any connected solvable linear algebraic group G acting on a scheme, the G-Chow group (made out of G-invariant cycles) is canonically the same as the ordinary Chow group. I think I knew this paper at one point, but then obviously forgot ;-)

[novoselt]

comment:21

1. The discussed above fact that this patch implement the full Chow group should be reflected in module level documentation.
   1. Documentation from ChowCycle.__init__ should be moved to the class docstring, otherwise it is invisible.
   2. Behaviour of check=False is strange for it: usually such an option omits validity checks assuming that user will take care of it. Here it seems to allow a different form of input. Is it inherited from the base class? If so, maybe it should be fixed there...
   3. Line 170 misses the second ":" in the end.
   4. The first words of some docstrings have extra s'es, like "Returns" and "Intersects".
   5. It would be nice if the documentation and example for count_points were expanded/clarified a little bit more. The phrase "integral over the dual cohomology class" does not make it clear what is the integrand and to what the cohomology class is dual. Is it the Chow cycle in both cases? The example starts with a divisor and then integrates the square of this divisor and counts points on the intersection of the Chow cycle of this divisor with the original divisor. How about starting with some Chow cycle with non-zero point count and then showing how to get a corresponding cohomology class and integrate it?
   6. For intersect_with_divisor it would be nice to give a reference to the used algorithm (Fulton p.97?). I think also that intersection_with_divisor would be a better name, since the function returns the intersection rather than updates the original cycle. (I do realize now that the same consideration should have been applied to some methods that I have added...) By the way, this function has no input/output description. And perhaps i = I_gamma.pop() does the same thing as i = iter(I_gamma).next() in a little cleaner fashion. (It does alter the set, but it is not used anyway.) What exactly is tested by the long test with many zeros in this function? This function says that divisor must be Cartier, but it is not checked, is it intended? Am I right that it actually makes sense to use it with Q-Cartier divisors if the Chow group is also rational?
   7. Chow_cycle method of toric divisors does not have a doctest and input/output description.
   8. Line 394 misses closing quotes after True.

I didn't go over actual Chow group code yet, but will do it. Soon ;-)

[novoselt]

comment:22

10. It is probably worth mentioning what is the point of A(Cone(cone)) after A(cone). I actually didn't even know that Cone(cone) works! Fortunately, it works as expected ;-)
    11. In _rational_equivalence_relations I find it very difficult to understand the LaTeX expression, which does not get typeset in the documentation (since it is a private method), especially since it does not explain what is "! over =", what is n_{\rho, \sigma}, and how d, k, n, and p are related (I guess pairs of them are equal). I think that this description should be moved to some visible place (module documentation or relation_gens?) and there should be a reference to where it was taken from.
    12. What is the purpose of the super call in __eq__ for Chow groups? Do we want anything but a Chow group equal to a Chow group?
    13. I think that _cone_to_V would be more efficient if it was implemented via a dictionary. I don't know how significant such a speed up will be in real computations, but I think it is worthwhile to do this change.
    14. There is a note in degree method about smooth varieties: "Using the cohomology ring instead of the Chow group will be much faster." I think that this is a very important point and it should be also mentioned in the module level documentation and in the Chow group method of toric varieties. (Perhaps with explanations of why this is the case.) Examples of this method are very nice! I'd remove the sentence "The resulting fan is not the fan over a convex cube."
    15. I think it would be more natural if gens without parameters returned the union of all gens of specified degree. Is it possible to implement? (I.e. is it possible to force the quotient module to use these generators? I definitely don't suggest "just" returning union of degree generators.)
    16. For relation_gens wouldn't it be more natural to return elements of the covering module, i.e. lifts of the current output?
    17. Line 979: you meant == QQ.
    18. Would it make sense to derive ChowGroup_degree_class from some kind of a group rather than SageObject?

That's it for the current patch!

[novoselt]

comment:23

One more global question: did you think about having most of the functionality in ChowGroup_degree_class with Chow cycles being tuples of elements from all of them, i.e. without having the total quotient module? That would take care of some of the issues above (like gens vs. gens(degree=k)) and would avoid flatten(fan.cones()) which I don't really like. It would also make sense to let fans determine indices of the cones in the lists in this case, i.e. fan._index_of(cone) would return the index of cone in fan.cones(dim=cone.dim()).

[vbraun]

comment:24

When I wrote the code I tried to make gens() return the union of the the fixed-degree generators but that did not work. I don't remember what exactly went wrong, but bad things happen if gens() is different from the generators that FGP_Module choses.

I mostly added ChowGroup_degree_class so that the individual degree pieces can _repr_() themselves nicely. One could make them parents again with some coercion in the full Chow group, but that seems to be a big effort. I definitely want to allow mixed-degree Chow group elements, so we can't get rid of the whole Chow group.

[vbraun]

comment:25

A1. The discussed above fact that this patch implement the full Chow group should be reflected in module level documentation.

OK

A2. Documentation from ChowCycle.__init__ should be moved to the class docstring, otherwise it is invisible.

I've added a note to the `ChowCycle` class documentation that you are not supposed to manually construct them. The `__init__` docstring is hidden on purpose.

A3. Behaviour of check=False is strange for it: usually such an option omits validity checks assuming that user will take care of it. Here it seems to allow a different form of input. Is it inherited from the base class? If so, maybe it should be fixed there...

Yes, it is inherited from `FGP_Element`. I tried to fix it there, but then I ended up trying to rewrite it for the new coercion model. And that attempt was quite unsuccessful, all of sage.modules is interrelated and you can't just change one class...

A4. Line 170 misses the second ":" in the end.

OK

A5. The first words of some docstrings have extra s'es, like "Returns" and "Intersects".

OK

A6. It would be nice if the documentation and example for count_points were expanded/clarified a little bit more. The phrase "integral over the dual cohomology class" does not make it clear what is the integrand and to what the cohomology class is dual. Is it the Chow cycle in both cases? The example starts with a divisor and then integrates the square of this divisor and counts points on the intersection of the Chow cycle of this divisor with the original divisor. How about starting with some Chow cycle with non-zero point count and then showing how to get a corresponding cohomology class and integrate it?

There is no easy way to get the Poincare dual (In fact, it exists only if the variety is smooth). I've clarified the example a bit.

A7. For intersect_with_divisor it would be nice to give a reference to the used algorithm (Fulton p.97?). I think also that intersection_with_divisor would be a better name, since the function returns the intersection rather than updates the original cycle. (I do realize now that the same consideration should have been applied to some methods that I have added...) By the way, this function has no input/output description. And perhaps i = _gamma.pop() does the same thing as i = iter(I_gamma).next() in a little cleaner fashion. (It does alter the set, but it is not used anyway.) What exactly is tested by the long test with many zeros in this function? This function says that divisor must be Cartier, but it is not checked, is it intended? Am I right that it actually makes sense to use it with Q-Cartier divisors if the Chow group is also rational?

I renamed it to `intersection_with_divisor()` and expanded on the documentation which should now answer your questions...

In principle you are right with the Q-Cartier divisors, but in practice this will not work because multiplication QQ * QQ-Chow cycle is broken. The problem is that the parent does not adhere to the new coercion model. I could hack around it, but then this should really be fixed elsewhere.

A8. Chow_cycle method of toric divisors does not have a doctest and input/output description.

OK

A9. Line 394 misses closing quotes after True.

OK

B1. It is probably worth mentioning what is the point of A(Cone(cone)) after A(cone). I actually didn't even know that Cone(cone) works! Fortunately, it works as expected ;-)

OK

B2. In _rational_equivalence_relations I find it very difficult to understand the LaTeX expression, which does not get typeset in the documentation (since it is a private method), especially since it does not explain what is "! over =", what is n_{\rho, \sigma}, and how d, k, n, and p are related (I guess pairs of them are equal). I think that this description should be moved to some visible place (module documentation or relation_gens?) and there should be a reference to where it was taken from.

I moved it to `relation_gens` and added more explanations.

B3. What is the purpose of the super call in __eq__ for Chow groups? Do we want anything but a Chow group equal to a Chow group?

Now it only returns `self is other`.

B4. I think that _cone_to_V would be more efficient if it was implemented via a dictionary. I don't know how significant such a speed up will be in real computations, but I think it is worthwhile to do this change.

Its only constructing a unit vector. Surely the run time for intersection computations is dominated by the matrix normal form computations and not the construction of unit vectors?

B5. There is a note in degree method about smooth varieties: "Using the cohomology ring instead of the Chow group will be much faster." I think that this is a very important point and it should be also mentioned in the module level documentation and in the Chow group method of toric varieties. (Perhaps with explanations of why this is the case.) Examples of this method are very nice! I'd remove the sentence "The resulting fan is not the fan over a convex cube."

OK

B6. I think it would be more natural if gens without parameters returned the union of all gens of specified degree. Is it possible to implement? (I.e. is it possible to force the quotient module to use these generators? I definitely don't suggest "just" returning union of degree generators.)

I tried it but couldn't get it to work. The base `FGP_Module` has no provision to explicitly set the generators, and bad things happen if they are different from the internal normal form of the basis matrix.

B7. For relation_gens wouldn't it be more natural to return elements of the covering module, i.e. lifts of the current output?

You can do that with `A.relations().gens()`. I added a note to `relation_gens`.

B8. Line 979: you meant == QQ.

Yes, fixed.

B9. Would it make sense to derive ChowGroup_degree_class from some kind of a group rather than SageObject?

Ideally they would be some other parents with coercions into the full (mixed-degree) Chow group. But then thats a lot of work for very little benefit. Not to mention that the base `FGP_Module` does not adhere to the new coercion model, so it'll be hackisch...

[novoselt]

comment:26

For the buildbot: Depends on #9972 and #10325

[vbraun]

comment:27

Wait, I'm still meddling with the coercions...

[novoselt]

comment:28

Replying to @vbraun:

A3. OK, let's hope it will get fixed eventually...

A6. Am I right that it is integral OF the dual cohomology class, not OVER? That's how I interpret integrate method of toric varieties. Also it seems that we need completeness in addition to smoothness.

A7. Let it be fixed elsewhere!

B9. Ok, let it be as is.

Fresh comments:

1. Why get_degree and not just degree for Chow cycles?
2. What will cohomology_class of a Chow cycle produce in the case when there is no Poincare duality? I think this should be either explained in the documentation, or an exception should be raised varieties which are not smooth and complete.

I have done some documentation prettification, feel free to fold it into your patch, if is still applies to your current version.

[novoselt]

Attachment: trac_9713_reviewer.patch.gz

[vbraun]

comment:29

A6. Yes, integral "of". Will fix it ;-)

1. ChowGroup.degree() is probably a stupid name... ideas for better names? But I wanted to distinguish it from ChowCycle.degree().

2. There is no ChowCycle.cohomology_class() method nor any coercion to the CohomologyRing.

[novoselt]

comment:30

Replying to @vbraun:

1. ChowGroup.degree() is probably a stupid name... ideas for better names? But I wanted to distinguish it from ChowCycle.degree().

I meant that ChowCycle.degree() is better than ChowCycle.get_degree() defined on line 243. It seems fine to me, why don't you like it?

2. There is no ChowCycle.cohomology_class() method nor any coercion to the CohomologyRing.

What about line 444?..

[vbraun]

comment:31

Oops I totally forgot that I implemented the cohomology_class() method :-)

I ported the bases Module and FGP_Module over to the new coercion model. The patch touches a lot of files but mostly small fixes, only sage/modules/module.py and sage/modules/fg_pid/*py are modified big time.

For the tracbot: Apply trac_9713_fix_cardinality.patch, trac_9713_fix_fg_pid.patch, trac_9713_toric_chow_group.patch

[simon-king-jena]

comment:33

I am about to create a new ticket whose purpose is to implement the new parent and coercion framework not only for the base class sage.modules.module.Module, but also for the most derived classes. In that regard, it goes beyond what is done in the patches here.

My to-be-created ticket will leave the FGP_* stuff almost unchanged. I suggest to include that to-be-created ticket as a new dependency (mentioning it in #9604 as well), so that the ticket here can concentrate on its "real" purpose, which is not coercion but toric Chow group.

Then I would also provide a replacement of trac_9713_toric_chow_group.patch relative to the to-be-created ticket.

A few words on trac_9713_toric_chow_group.patch: The element_class attribute of Parent is in fact a lazy attribute, that takes another attribute Element and mixes it with some category stuff before it turns element_class into an actual attribute. Hence, one should not directly set element_class (which is done in the patch), but should provide an attribute Element instead.

My to-be-created patch would actually preserve the _element_class (leading underscore) method, since it chooses the right element class and is already implemented for all sub-classes. But I would call it in the __init__ method of the base class sage.modules.module.Module and assign its output to the Element attribute. In that way, it is most easy to implement the lazy element_class (no leading underscore) attribute as one is supposed to.

Also, the __call__ method of fgp morphisms should probably be replaces by _call_, _call_with_args and perhaps pushforward (likely depending on #10496).

Best regards, Simon

[simon-king-jena]

Attachment: trac_9713_fix_fg_pid_relative_10513.patch.gz

Rebases trac_9713_fix_fg_pid.patch relative to #10513

[simon-king-jena]

comment:34

I just attached a version of trac_9713_fix_fg_pid.patch that is based on #10513. I'll add #10513 to the dependencies that are listed in #9604.

I can not vouch for any passing doc tests, as I merely rebased the patch.

[simon-king-jena]

comment:35

Sorry! Since #10513 needs much work, I should not make it a dependency for the ticket here. So, please disregard the patch trac_9713_fix_fg_pid_relative_10513.patch!

I understood that you just want to implement the parent and category framework to an extent that allows you to implement the toric Chow group. Right? Then I think my remark on _call_ and pushforward and _call_with_args of morphisms should be moved to a different ticket.

So, it remains "needs review".

Cheers, Simon

[novoselt]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
 The toric Chow group is a useful version of a homology theory for toric varieties, especially for singular varieties. The attached patch provides support for computing the toric Chow group as well as elementary intersection theory.

-See tracker bug at #9604 to for the patch queue/dependencies.
+See tracker bug at #9604 to for related patches.

[novoselt]

comment:36

Just to make it clear: so far this ticket depends on #9972 and #10325 (both positively reviewed) on top of sage-4.6.1.alpha3, nothing else.

[simon-king-jena]

comment:37

Replying to @novoselt:

Just to make it clear: so far this ticket depends on #9972 and #10325 (both positively reviewed) on top of sage-4.6.1.alpha3, nothing else.

Thank you for the clarification! I thought I had to apply all patches mentioned at #9604.

[vbraun]

Attachment: trac_9713_fix_fg_pid.patch.gz

Renamed element_class -> Element

[vbraun]

comment:38

I've renamed element_class -> Element as per Simon's comment, thanks!

Maybe Simon would be interested in reviewing the first two patches (trac_9713_fix_cardinality.patch, trac_9713_fix_fg_pid.patch) and Andrey the trac_9713_toric_chow_group.patch? :-) Then we should make it into Sage-4.6.2...

[simon-king-jena]

comment:39

Replying to @vbraun:

I've renamed element_class -> Element as per Simon's comment, thanks!

Maybe Simon would be interested in reviewing the first two patches (trac_9713_fix_cardinality.patch, trac_9713_fix_fg_pid.patch) and Andrey the trac_9713_toric_chow_group.patch? :-) Then we should make it into Sage-4.6.2...

In what order should the patches be applied? I first applied the patches from #9972 and #10325, then proceeded with trac_9713_toric_chow_group.patch, but one hunk of trac_9713_fix_fg_pid.patch failed.

[vbraun]

comment:40

See also comment 31:

• trac_9713_fix_cardinality.patch
• trac_9713_fix_fg_pid.patch
• trac_9713_toric_chow_group.patch

[simon-king-jena]

comment:41

Replying to @vbraun:

See also comment 31:

• trac_9713_fix_cardinality.patch
• trac_9713_fix_fg_pid.patch

No, when I apply trac_9713_fix_fg_pid.patch, one out of 17 hunks fails, namely:

--- fgp_module.py
+++ fgp_module.py
@@ -539,7 +578,8 @@
         """
         return other.is_submodule(self)

-    def __call__(self, x, check=True):
+
+    def _element_constructor_(self, x, check=True):
         """
         INPUT:

Cheers, Simon

[vbraun]

comment:42

Works for me, which sage version are you trying to apply it to?

/home/vbraun/opt/sage-4.6.1.rc0/devel/sage
[vbraun@volker-desktop sage]$ hg qpush -a
applying trac_9972_add_cone_embedding.patch
applying trac_9972_improve_element_constructors.patch
applying trac_9972_remove_enhanced_cones_and_fans.patch
applying trac_9972_add_fan_morphisms.patch
applying trac_9972_fix_fan_warning.patch
applying trac_10325_make_toric_CohomologyRing_unique.patch
applying trac_9713_fix_cardinality.patch
applying trac_9713_fix_fg_pid.patch
applying trac_9713_toric_chow_group.patch
now at: trac_9713_toric_chow_group.patch

[simon-king-jena]

comment:43

Replying to @simon-king-jena:

No, when I apply trac_9713_fix_fg_pid.patch, one out of 17 hunks fails

It seems to me that this hunk has a totally wrong line number. The line

    def __call__(self, x, check=True):

is line number 446 in fgp_module.py.

[vbraun]

comment:44

No the __call__ in FGP_Module is at line 542. Maybe you forgot to revert something in your sage library?

[vbraun@volker-desktop sage]$ hg qpop -a
no patches applied
[vbraun@volker-desktop sage]$ head -542 sage/modules/fg_pid/fgp_module.py | tail -1
    def __call__(self, x, check=True):
[vbraun@volker-desktop sage]$