Opened 12 years ago
Last modified 7 years ago
#9466 closed defect
square root with all=True should not return ValueError but empty list — at Version 5
Reported by: | mstreng | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-5.12 |
Component: | algebra | Keywords: | sd23 sd51 sqrt all |
Cc: | mderickx, ruckers | Merged in: | |
Authors: | Marco Streng | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
With Sage 4.4.4 and no relevant patches, I got the following:
sage: FiniteField(3)(2).sqrt(all = True) [] sage: 2.sqrt(extend = False, all = True) ValueError: square root of 2 not an integer sage: FiniteField(next_prime(2^100))(2).sqrt(extend = False, all = True) ValueError: self must be a square sage: _.<a>=FiniteField(9) sage: a.sqrt(extend = False, all = True) ValueError: must be a perfect square.
At sage days 23 we agreed that square root with all=True should not raise an error. If no square roots exist, then it should return an empty list.
Right now, it returns an empty list for elements of small prime finite fields, but raises an error for integers, elements of large prime finite fields, and elements of non-prime finite fields.
apply
Change History (10)
comment:1 Changed 12 years ago by
- Description modified (diff)
Changed 10 years ago by
Changed 10 years ago by
Fixes the code: sage: FiniteField?(next_prime(2^{100))(2).sqrt(extend = False, all = True) }
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
I was not able to fix this code.
sage: _.<a>=FiniteField?(9)
sage: a.sqrt(extend = False, all = True)
ValueError?: must be a perfect square.
comment:4 Changed 10 years ago by
Thanks! That bug has been around for too long :)
Why were you not able to fix the FiniteField(9)
case?
Unfortunately, the patches need some more work, for the following three reasons.
- With my2.patch:
sage: Zmod(7)(3).sqrt(extend=True) sqrt3 sage: Zmod(7)(3).sqrt(all=True, extend=True) []
The second one should either output[sqrt3, -sqrt3]
or raiseNotImplementedError
, but never an empty list. - Patches must have your name and email address in them. This is done by putting your email address and name in your
.hgrc
file as specified here - Add an example of the new functionality to the documentation:
- This helps the user understand what the function does.
- Through the doctesting framework, this prevents other people from accidentally breaking your added functionality.
- This is required for your patch to be able to get a positive review. In fact, the manual says that you should also mention the ticket number #9466 with your example.
If you have any questions, let me know!
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
comment:5 Changed 8 years ago by
- Cc ruckers added
- Description modified (diff)
- Keywords sd23 sd51 sqrt all added
Does anyone know who user "ruckers" is? (s)he should be added to the list of authors
apply 9466.patch
Fixes the code: sage: 2.sqrt(extend = False, all = True)