Opened 5 years ago
Closed 5 years ago
#19318 closed enhancement (wontfix)
Implement inverse_of_unit for IntegerModRing
Reported by: | bruno | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | finite rings | Keywords: | |
Cc: | Merged in: | ||
Authors: | Bruno Grenet | Reviewers: | Kwankyu Lee |
Report Upstream: | N/A | Work issues: | |
Branch: | u/bruno/group_inverse_of_unit (Commits) | Commit: | c626f8cbf02953b94a2ca54dc0599553c0f40b47 |
Dependencies: | Stopgaps: |
Description
This (small) patch allows one to compute the inverse of a unit in an IntegerModRing
. It provides the following functionality:
sage: R = IntegerModRing(10) sage: R(7).inverse_of_unit() 3 sage: R(4).inverse_of_unit() # 4 is not a unit in ZZ/10ZZ Traceback (most recent call last): ... ZeroDivisionError: Inverse does not exist.
Change History (9)
comment:1 Changed 5 years ago by
- Branch set to u/bruno/group_inverse_of_unit
comment:2 Changed 5 years ago by
- Commit set to c626f8cbf02953b94a2ca54dc0599553c0f40b47
- Status changed from new to needs_review
comment:3 follow-up: ↓ 4 Changed 5 years ago by
Why do we need this? You can use ^-1
for the same result.
sage: R=IntegerModRing(10) sage: R(7)^-1 3 sage: R(4)^-1 --------------------------------------------------------------------------- ZeroDivisionError Traceback (most recent call last) ... ZeroDivisionError: Inverse does not exist.
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 5 years ago by
Replying to klee:
Why do we need this? You can use
^-1
for the same result.sage: R=IntegerModRing(10) sage: R(7)^-1 3 sage: R(4)^-1 --------------------------------------------------------------------------- ZeroDivisionError Traceback (most recent call last) ... ZeroDivisionError: Inverse does not exist.
That's right... I did not think to try to use ^-1
and I found weird not to have an inverse
function for these rings. And using ^-1
is actually faster than my code! So it makes no sense to include my code within Sage. Yet, since many rings implement a method inverse_of_unit
, it may add some unity (or consistency) to have one here too. So please let me know what you think is more appropriate between:
- simply marking this ticket as "won't fix" and not providing the method
inverse_of_unit
; - replacing my code by something like
return self^-1
to provide a methodinverse_of_unit
.
Note that both are fine to me, I do not have a real preference for any of the two solutions.
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 5 years ago by
- Milestone changed from sage-6.9 to sage-duplicate/invalid/wontfix
Replying to bruno:
- replacing my code by something like
return self^-1
to provide a methodinverse_of_unit
.
As this inverse_of_unit
does not implement a special algorithm to compute the inverse, it should not be a method of IntegerModRing
. On the other hand, adding inverse_of_unit
method to a general ring as a more intuitive alternative to ^-1
seems not a good idea. Is it really more intuitive? I would suggest multiplicative_inverse
. Anyway, this is a separate issue.
I advise you to ask in Sage support or Sage devel first if you think you found something missing in Sage.
I will mark this ticket as "won't fix"
comment:6 in reply to: ↑ 5 Changed 5 years ago by
Replying to klee:
Replying to bruno:
- replacing my code by something like
return self^-1
to provide a methodinverse_of_unit
.As this
inverse_of_unit
does not implement a special algorithm to compute the inverse, it should not be a method ofIntegerModRing
. On the other hand, addinginverse_of_unit
method to a general ring as a more intuitive alternative to^-1
seems not a good idea. Is it really more intuitive? I would suggestmultiplicative_inverse
. Anyway, this is a separate issue.I advise you to ask in Sage support or Sage devel first if you think you found something missing in Sage.
I will mark this ticket as "won't fix"
OK, that's fine!
comment:7 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:8 Changed 5 years ago by
- Reviewers set to Kwankyu Lee
comment:9 Changed 5 years ago by
- Resolution set to wontfix
- Status changed from positive_review to closed
New commits:
#19318: Implement inverse_of_unit for IntegerModRing