Opened 7 years ago
Closed 3 years ago
#10732 closed defect (fixed)
computing support of sum of two divisors doesn't work due to careless error
Reported by:  was  Owned by:  AlexGhitza 

Priority:  major  Milestone:  sage6.2 
Component:  algebraic geometry  Keywords:  
Cc:  Merged in:  
Authors:  Alex Ghitza  Reviewers:  Peter Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  4e4716a (Commits)  Commit:  4e4716a3d758dbec59b2a5c040e15516f33833a2 
Dependencies:  Stopgaps: 
Description (last modified by )
sage: R.<x,y,z> = GF(5)[]; C = Curve(x^7 + y^7 + z^7) sage: pts = C.rational_points() sage: D = C.divisor([(2,pts[0])]) sage: D.support() [(0 : 4 : 1)] sage: (D+D).support() Traceback (most recent call last):...
Change History (15)
comment:1 Changed 7 years ago by
 Description modified (diff)
comment:2 Changed 4 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:3 Changed 4 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:4 Changed 3 years ago by
 Branch set to u/AlexGhitza/ticket/10732
 Modified changed from 01/30/14 21:20:52 to 01/30/14 21:20:52
comment:5 Changed 3 years ago by
 Commit set to 3496b12c2771d25f6e5c54c46e7120b77bc19e40
 Status changed from new to needs_review
comment:6 Changed 3 years ago by
 Status changed from needs_review to needs_work
It should be This checks that :trac:
and not This checks that trac::
comment:7 Changed 3 years ago by
 Commit changed from 3496b12c2771d25f6e5c54c46e7120b77bc19e40 to 375d57301c07ce04a7e90809a57bc958a38b10c6
Branch pushed to git repo; I updated commit sha1. New commits:
375d573  docstring fix

comment:8 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 3 years ago by
I haven't looked at this in detail, but you use the method rational_points()
of the subscheme defined by the divisor. Wouldn't irreducible_components()
be the right thing to use, to handle the case where the support contains nonrational points?
comment:10 followup: ↓ 11 Changed 3 years ago by
I agree that this would be better, however Sage can't yet deal with divisors containing nonrational points (see the TODO in the docstring of Divisor_curve
in sage/schemes/generic/divisor.py
).
How about a comment in the new code saying that we should use irreducible_components()
as soon as nonrational points can be dealt with? I'd rather not postpone fixing an existing issue while we wait for new functionality that could take a while to arrive.
comment:11 in reply to: ↑ 10 ; followup: ↓ 12 Changed 3 years ago by
 Reviewers set to Peter Bruin
Replying to AlexGhitza:
I agree that this would be better, however Sage can't yet deal with divisors containing nonrational points (see the TODO in the docstring of
Divisor_curve
insage/schemes/generic/divisor.py
).
Wow, and there doesn't even seem to be a ticket for it! I'm not sure if I agree with the TODO; I would say a divisor is a sum of prime divisors (closed points of the scheme), and if you want arbitrary sums of Lrational points you have to base change to L.
How about a comment in the new code saying that we should use
irreducible_components()
as soon as nonrational points can be dealt with? I'd rather not postpone fixing an existing issue while we wait for new functionality that could take a while to arrive.
That is a good idea. Could you also delete the trailing whitespace in the first empty line after the function? Then you can set it to positive review.
comment:12 in reply to: ↑ 11 Changed 3 years ago by
comment:13 Changed 3 years ago by
 Commit changed from 375d57301c07ce04a7e90809a57bc958a38b10c6 to 4e4716a3d758dbec59b2a5c040e15516f33833a2
Branch pushed to git repo; I updated commit sha1. New commits:
4e4716a  comment about nonrational points in divisors

comment:14 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:15 Changed 3 years ago by
 Branch changed from u/AlexGhitza/ticket/10732 to 4e4716a3d758dbec59b2a5c040e15516f33833a2
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
fix support of divisors obtained via arithmetic