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 bruno

  • Branch set to u/bruno/group_inverse_of_unit

comment:2 Changed 5 years ago by bruno

  • Commit set to c626f8cbf02953b94a2ca54dc0599553c0f40b47
  • Status changed from new to needs_review

New commits:

c626f8c#19318: Implement inverse_of_unit for IntegerModRing

comment:3 follow-up: Changed 5 years ago by 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.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 5 years ago by bruno

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 method inverse_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: Changed 5 years ago by klee

  • 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 method inverse_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 bruno

Replying to klee:

Replying to bruno:

  • replacing my code by something like return self^-1 to provide a method inverse_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"

OK, that's fine!

comment:7 Changed 5 years ago by klee

  • Status changed from needs_review to positive_review

comment:8 Changed 5 years ago by klee

  • Reviewers set to Kwankyu Lee

comment:9 Changed 5 years ago by vbraun

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.