Ticket #5962 (closed defect: fixed)

Opened 4 years ago

Last modified 2 years ago

Comparison in the Gap interface raises an error

Reported by: SimonKing Owned by: was
Priority: major Milestone: sage-4.7
Component: interfaces Keywords: gap comparison
Cc: wdj Work issues:
Report Upstream: N/A Reviewers: David Joyner
Authors: Simon King Merged in: sage-4.7.alpha3
Dependencies: Stopgaps:

Description

On sage.math with sage-3.4.1, one has

sage: gap('DihedralGroup(8)')==gap('DihedralGroup(8)')
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
...
RuntimeError: Gap produced error output
Error, no 1st choice method found for `LT' on 2 arguments

   executing $sage1 < $sage2;

The problem seems to be that Gap is unable to compare:

sage: gap('DihedralGroup(8)=DihedralGroup(8)')
false

Perhaps it would make sense to try and implement a __cmp__ method that is more sophisticated than what is done in Gap?

At least it should be made sure that the __cmp__ method of the Gap interface does not raise an error.

Attachments

trac_5962_GAP__cmp__.patch Download (2.6 KB) - added by SimonKing 2 years ago.
Avoid an error being raised when comparing GAP elements. Add doctest.

Change History

comment:1 Changed 3 years ago by SimonKing

  • Status changed from new to needs_review
  • Report Upstream set to N/A
  • Authors set to Simon King

I see no way for a really satisfying solution, as long as GAP can not even compare two objects whose definitions are identical.

However, the errors being raised by GAP when comparing elements are now caught in a try-except clause. We have, as doc tests:

sage: gap('DihedralGroup(8)')==gap('DihedralGroup(8)')
False    # sorry, this is what GAP claims.
sage: gap('SymmetricGroup(8)')<gap('AlternatingGroup(8)')
True
sage: gap('SymmetricGroup(8)')>gap('AlternatingGroup(8)')
False
sage: gap('SymmetricGroup(8)')==gap('SymmetricGroup(8)')
True

All but the first of these examples worked before. But the first resulted in an error, which is now fixed.

comment:2 follow-up: ↓ 3 Changed 2 years ago by SimonKing

I just found that this ticket needs review since 8 months. Fortunately the patch still works fine. So, any volunteer?

comment:3 in reply to: ↑ 2 Changed 2 years ago by wdj

Replying to SimonKing:

I just found that this ticket needs review since 8 months. Fortunately the patch still works fine. So, any volunteer?

I have spring break coming up and can try to review it then if no one beats me to it.

comment:4 Changed 2 years ago by wdj

  • Status changed from needs_review to positive_review

This patch applies to 4.7.a1 and passes sage -testall. The patch does as claimed (adding some try-except statements) and contains the proper additional examples in the docstrings. Positive review from me.

Thanks Simon!

comment:5 follow-up: ↓ 6 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to needs_work
  • Reviewers set to David Joyner

Please change the commit message (using hg qrefresh -e) such that the ticket number appears on the first line.

Changed 2 years ago by SimonKing

Avoid an error being raised when comparing GAP elements. Add doctest.

comment:6 in reply to: ↑ 5 ; follow-up: ↓ 7 Changed 2 years ago by SimonKing

  • Status changed from needs_work to positive_review

Replying to jdemeyer:

Please change the commit message (using hg qrefresh -e) such that the ticket number appears on the first line.

Done.

comment:7 in reply to: ↑ 6 Changed 2 years ago by jdemeyer

Replying to SimonKing:

Done.

Thanks!

comment:8 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.7.alpha3
Note: See TracTickets for help on using tickets.