Opened 4 years ago
Closed 13 months 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: | sage-6.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 was)
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 4 years ago by was
- Description modified (diff)
comment:2 Changed 22 months ago by jdemeyer
- Milestone changed from sage-5.11 to sage-5.12
comment:3 Changed 16 months ago by vbraun_spam
- Milestone changed from sage-6.1 to sage-6.2
comment:4 Changed 13 months ago by AlexGhitza
- Branch set to u/AlexGhitza/ticket/10732
- Modified changed from 01/30/14 13:20:52 to 01/30/14 13:20:52
comment:5 Changed 13 months ago by AlexGhitza
- Commit set to 3496b12c2771d25f6e5c54c46e7120b77bc19e40
- Status changed from new to needs_review
comment:6 Changed 13 months ago by chapoton
- Status changed from needs_review to needs_work
It should be This checks that :trac: and not This checks that trac::
comment:7 Changed 13 months ago by git
- Commit changed from 3496b12c2771d25f6e5c54c46e7120b77bc19e40 to 375d57301c07ce04a7e90809a57bc958a38b10c6
Branch pushed to git repo; I updated commit sha1. New commits:
375d573 | docstring fix |
comment:8 Changed 13 months ago by AlexGhitza
- Status changed from needs_work to needs_review
comment:9 Changed 13 months ago by pbruin
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 non-rational points?
comment:10 follow-up: ↓ 11 Changed 13 months ago by AlexGhitza
I agree that this would be better, however Sage can't yet deal with divisors containing non-rational 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 non-rational 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 ; follow-up: ↓ 12 Changed 13 months ago by pbruin
- Reviewers set to Peter Bruin
Replying to AlexGhitza:
I agree that this would be better, however Sage can't yet deal with divisors containing non-rational points (see the TODO in the docstring of Divisor_curve in sage/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 L-rational 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 non-rational 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 13 months ago by pbruin
comment:13 Changed 13 months ago by git
- Commit changed from 375d57301c07ce04a7e90809a57bc958a38b10c6 to 4e4716a3d758dbec59b2a5c040e15516f33833a2
Branch pushed to git repo; I updated commit sha1. New commits:
4e4716a | comment about non-rational points in divisors |
comment:14 Changed 13 months ago by AlexGhitza
- Status changed from needs_review to positive_review
comment:15 Changed 13 months ago by vbraun
- Branch changed from u/AlexGhitza/ticket/10732 to 4e4716a3d758dbec59b2a5c040e15516f33833a2
- Resolution set to fixed
- Status changed from positive_review to closed
New commits: