Opened 3 years ago
Closed 3 years ago
#28404 closed defect (fixed)
Sign of hypergeometric motives is sometimes wrong
Reported by: | kedlaya | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.9 |
Component: | modular forms | Keywords: | hypergeometric motives |
Cc: | Merged in: | ||
Authors: | Kiran Kedlaya | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | 37fda46 (Commits, GitHub, GitLab) | Commit: | 37fda46afe7e05a2fac93ec6a6e6edfd5b67ad4d |
Dependencies: | Stopgaps: |
Description
The sign
method of HypergeometricData
sometimes returns answers that do not agree with Magma:
sage: from sage.modular.hypergeometric_motive import HypergeometricData as Hyp sage: H = Hyp(cyclotomic=([6,2],[1,1,1])) sage: [H.sign(1/4,p) for p in [5,7,11,13,17,19]] #correct [1, 1, -1, -1, 1, 1] sage: H = Hyp(cyclotomic=([1,1,1],[6,2])) sage: [H.sign(4,p) for p in [5,7,11,13,17,19]] #should agree but doesn't [-1, -1, -1, -1, -1, -1]
I believe this is because Magma's formula is not valid when 0 in alpha
. Note that this does not affect the padic_H_value
or euler_factor
methods because these do an alpha-beta swap to avoid this case (just as in Magma).
I have a fix for this coded up locally. Will post a patch shortly.
Change History (8)
comment:1 Changed 3 years ago by
- Branch set to u/kedlaya/sign_of_hypergeometric_motives_is_sometimes_wrong
comment:2 Changed 3 years ago by
- Commit set to 8bbce1f713dfec2971a14433d3e629195bbeb1dd
comment:3 Changed 3 years ago by
Right, it is probably a bad idea to not return what is asked for. I understand that Magma originally did this and it caused all sorts of headaches.
A better idea might be, at creation time, if 0 in alpha
, instantiate the swapped data and save it in H._swap
. Then one can access this attribute when sign
is called (or for that matter padic_H_value
or H_value
or euler_factor
).
comment:4 Changed 3 years ago by
- Commit changed from 8bbce1f713dfec2971a14433d3e629195bbeb1dd to 68eeea1a521543804b1f8636e877199d6fd5be30
Branch pushed to git repo; I updated commit sha1. New commits:
68eeea1 | Add _swap attribute to HypergeometricData
|
comment:5 Changed 3 years ago by
- Status changed from new to needs_review
While implementing this last suggestion, I discovered I was getting strange irreproducible errors if I didn't perform
t = QQ(t)
before doing the swap (the value of t was sometimes getting cast to 0).
comment:6 Changed 3 years ago by
- Branch changed from u/kedlaya/sign_of_hypergeometric_motives_is_sometimes_wrong to public/ticket/28404
- Commit changed from 68eeea1a521543804b1f8636e877199d6fd5be30 to 37fda46afe7e05a2fac93ec6a6e6edfd5b67ad4d
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
comment:7 Changed 3 years ago by
Looks good, thanks.
comment:8 Changed 3 years ago by
- Branch changed from public/ticket/28404 to 37fda46afe7e05a2fac93ec6a6e6edfd5b67ad4d
- Resolution set to fixed
- Status changed from positive_review to closed
Maybe we should instead always switch when creating the object (in the
__init__
) when0 in alpha
?New commits:
Fix sign of hypergeometric motives
Docstring edit
EDIT:
But maybe this is not a good idea, because the user will not get what he wants..