Opened 9 years ago
Closed 9 years ago
#14900 closed defect (fixed)
Normalization for modular_symbols is wrong
Reported by: | was | Owned by: | cremona |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.12 |
Component: | elliptic curves | Keywords: | |
Cc: | wuthrich | Merged in: | sage-5.12.beta3 |
Authors: | Chris Wuthrich | Reviewers: | William Stein |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The docstring for E.modular_symbol
accidentally includes "normalize" twice. I think the *first* instance, which a user might read first, is wrong and should be deleted (it must have been accidentally left there).
sage: E = EllipticCurve('11a') sage: E.modular_symbol? ... - ``normalize`` - (default: True); if True, the modular symbol is correctly normalized (up to possibly a factor of -1 or 2). If False, the modular symbol is almost certainly not correctly normalized, i.e., all values will be a fixed scalar multiple of what they should be. But the initial computation of the modular symbol is much faster, though evaluation of it after computing it won't be any faster. - ``use_eclib`` - (default: False); if True the computation is done with John Cremona's implementation of modular symbols in ``eclib`` - ``normalize`` - (default: 'L_ratio'); either 'L_ratio', 'period', or 'none'; For 'L_ratio', the modular symbol is correctly normalized as explained above by comparing it to ``L_ratio`` for the curve and some small twists. The normalization 'period' is only available if ...
Apply the patch "trac_14900_modsymdoc.patch".
Attachments (1)
Change History (6)
comment:1 Changed 9 years ago by
- Cc wuthrich added
comment:2 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
Yes, that is what happened an I am sorry. I submit a quick patch here that fixes this and a small other issue with this normalisation. If L_ratio fails it now falls back to period. The cases where this makes a real difference, like 1369b1 are too long to be included as a doctest.
In any case, I am writing on a new implementation of modular symbols which will change this normalisation business radically anyway.
comment:3 Changed 9 years ago by
- Status changed from needs_review to positive_review
Looks good to me. Thanks!
comment:4 Changed 9 years ago by
- Reviewers set to William Stein
- Summary changed from mistake in docstring for modular_symbols to Normalization for modular_symbols is wrong
comment:5 Changed 9 years ago by
- Merged in set to sage-5.12.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
I think you are correct, including the reason why the old version is there. I hope that Chris Wuthrich will confirm.