Opened 4 years ago

Closed 8 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 17 months ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:3 Changed 11 months ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 8 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 8 months ago by AlexGhitza

  • Authors set to Alex Ghitza
  • Commit set to 3496b12c2771d25f6e5c54c46e7120b77bc19e40
  • Status changed from new to needs_review

New commits:

3496b12fix support of divisors obtained via arithmetic

comment:6 Changed 8 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 8 months ago by git

  • Commit changed from 3496b12c2771d25f6e5c54c46e7120b77bc19e40 to 375d57301c07ce04a7e90809a57bc958a38b10c6

Branch pushed to git repo; I updated commit sha1. New commits:

375d573docstring fix

comment:8 Changed 8 months ago by AlexGhitza

  • Status changed from needs_work to needs_review

comment:9 Changed 8 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: Changed 8 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: Changed 8 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 8 months ago by pbruin

Replying to pbruin:

there doesn't even seem to be a ticket for it

This is now #16225.

comment:13 Changed 8 months ago by git

  • Commit changed from 375d57301c07ce04a7e90809a57bc958a38b10c6 to 4e4716a3d758dbec59b2a5c040e15516f33833a2

Branch pushed to git repo; I updated commit sha1. New commits:

4e4716acomment about non-rational points in divisors

comment:14 Changed 8 months ago by AlexGhitza

  • Status changed from needs_review to positive_review

comment:15 Changed 8 months ago by vbraun

  • Branch changed from u/AlexGhitza/ticket/10732 to 4e4716a3d758dbec59b2a5c040e15516f33833a2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.