Fix divisor_of_function for affine_curve and projective_curve
#40029
+301
−85
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grhkm21
suggested changes
May 1, 2025
| coefficients and points. (TODO: This will change to a more | ||
| structural output in the future, so | ||
| intersection_number should disappear in the future.) | ||
|
|
|
|
||
| OUTPUT: | ||
|
|
||
|
|
| sage: C.divisor_of_function(r) | ||
| [(6, (0, 0))] | ||
| """ | ||
|
|
Comment on lines
+324
to
+327
| - ``list`` - The divisor of r represented as a list of | ||
| coefficients and points. (TODO: This will change to a more | ||
| structural output in the future, so | ||
| intersection_number should disappear in the future.) |
There was a problem hiding this comment.
Suggested change
| - ``list`` - The divisor of r represented as a list of | |
| coefficients and points. (TODO: This will change to a more | |
| structural output in the future, so | |
| intersection_number should disappear in the future.) | |
| - ``list`` - The divisor of r represented as a list of | |
| coefficients and points. | |
| .. TODO:: | |
| This will change to a more structural output in the future, so ``intersection_number`` should disappear in the future. |
Please wrap this line for me, as GitHub review interface is not very good. .. TODO:: is a Sphinx directive for the documentation :)
Comment on lines
+353
to
+359
| OUTPUT: | ||
|
|
||
|
|
||
| - ``int`` - The multiplicity of intersection of P and Q at pt | ||
|
|
||
|
|
||
| EXAMPLES:: |
There was a problem hiding this comment.
Suggested change
| OUTPUT: | |
| - ``int`` - The multiplicity of intersection of P and Q at pt | |
| EXAMPLES:: | |
| OUTPUT: | |
| - ``int`` - The multiplicity of intersection of P and Q at pt | |
| EXAMPLES:: |
Comment on lines
+395
to
+397
| if r.numerator() == 1: | ||
| pass | ||
| else : |
There was a problem hiding this comment.
Suggested change
| if r.numerator() == 1: | |
| pass | |
| else : | |
| if r.numerator() != 1: |
| for a in Ab: | ||
| for b in Or: | ||
| if P(a,b) == 0 and Q(a,b) == 0: | ||
| In = In + [(intersection_number(P,Q,(a,b)),P2(a,b))] |
There was a problem hiding this comment.
Suggested change
| In = In + [(intersection_number(P,Q,(a,b)),P2(a,b))] | |
| In.append((P2(a, b), intersection_number(P, Q, (a, b)))) |
For your information, + [...] is very slow, .append can be used for appending a single element to the end of a list, or .extend for multiple.
There was a problem hiding this comment.
I swap the elements for the change below
| for a in Ab: | ||
| for b in Or: | ||
| if P(a,b) == 0 and Q(a,b) == 0: | ||
| Jn = Jn + [(-intersection_number(P,Q,(a,b)),P2(a,b))] |
There was a problem hiding this comment.
Suggested change
| Jn = Jn + [(-intersection_number(P,Q,(a,b)),P2(a,b))] | |
| Jn.append((P2(a, b), -intersection_number(P, Q, (a, b)))) | |
Comment on lines
+432
to
+441
| dct1 = {e[1]: e[0] for e in In} | ||
| dct2 = {e[1]: e[0] for e in Jn} | ||
| for e in dct2.keys(): | ||
| if e in dct1.keys(): | ||
| dct1[e] += dct2[e] | ||
| if dct1[e] == 0: | ||
| del dct1[e] | ||
| else: | ||
| dct1[e] = dct2[e] | ||
| res = [(e[1], e[0]) for e in dct1.items()] |
There was a problem hiding this comment.
Suggested change
| dct1 = {e[1]: e[0] for e in In} | |
| dct2 = {e[1]: e[0] for e in Jn} | |
| for e in dct2.keys(): | |
| if e in dct1.keys(): | |
| dct1[e] += dct2[e] | |
| if dct1[e] == 0: | |
| del dct1[e] | |
| else: | |
| dct1[e] = dct2[e] | |
| res = [(e[1], e[0]) for e in dct1.items()] | |
| dct1 = dict(In) | |
| dct2 = dict(Jn) | |
| res = [(dct1.get(key, 0) + dct2.get(key, 0), key) for key in dct1.keys() | dct2.keys()] | |
| res = [(val, key) for val, key in res if val != 0] |
Explanation (I don't actually know how much Python you know >_<):
- Calling
dicton a list of tuples will give you a dict automatically. Sodict([(1, 2), (3, 5)]) = {1: 2, 3: 5} - Your for loop is "merging" the two dicts, so I did that in the list comprehension. the
.keys()output allows you to do bitwise operation (they are basically sets) so... | ...enumerates over both keys without duplication. dct.get(key, 0)is the same asdct[key] if key in dct else 0.- Finally remove the 0 multiplicity divisors
You can make this even more compact by something like
res = [(val, key) for key in dct1.keys() | dct2.keys() if (val := dct1.get(key, 0) + dct2.get(key, 0)) != 0]But I prefer readability.
| pass | ||
| else : | ||
| Q0 = P0.parent()(r.numerator().homogenize(z)) | ||
| P = P0.subs(x=1);Q = Q0.subs(x=1) |
There was a problem hiding this comment.
I think this line errors if the coordinate ring of C doesn't use "x, y, z" as variable names. Can you test it? You can try with
A.<u, v, w> = ProjectiveSpace(GF(5), 2)
C = A.curve(u^3 * w + u * w^3 - v^4)
|
I don't know what you discussed about "divisors of functions", but you should be at least aware of the existing machinery of "divisors" implemented for function fields of integral curves (affine or projective). sage: F = GF(5)
sage: P2 = AffineSpace(2, F, names='xy')
sage: R = P2.coordinate_ring()
sage: x, y = R.gens()
sage: f = y^2 - x^9 - x
sage: C = Curve(f)
sage: K = FractionField(R)
sage: r = 1/x
sage: f = C.function(r)
sage: f.divisor()
2*Place (1/x, 1/x^5*y)
- 2*Place (x, y)
sage: D = _
sage: D.degree()
0
sage: D.basis_function_space() # Riemann-Roch space L(D)
[x]
sage: g = D.basis_function_space()[0]
sage: f*g
1
sage: x1, x2 = C.coordinate_functions()
sage: x1.divisor()
-2*Place (1/x, 1/x^5*y)
+ 2*Place (x, y)
sage: x2.divisor()
-9*Place (1/x, 1/x^5*y)
+ Place (x, y)
+ Place (x^4 + 2, y)
+ Place (x^4 + 3, y)
sage: D = _
sage: D.support()
[Place (1/x, 1/x^5*y), Place (x, y), Place (x^4 + 2, y), Place (x^4 + 3, y)]
sage: P = _[1] # a rational place
sage: P
Place (x, y)
sage: p = C.place_to_closed_point(P)
sage: p
Point (x, y)
sage: p.rational_point()
(0, 0)
sage: pt = _
sage: pt.closed_point()
Point (x, y)
sage: _.place()
Place (x, y)
sage: _ == P
True |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes the definition of
divisor_of_functionwhich has never been implemented, for eitheraffine_curveorprojective_curve.#sd123