Opened 2 years ago

Closed 2 years ago

#23915 closed defect (fixed)

include the inner_product_matrix in module comparison

Reported by: sbrandhorst Owned by:
Priority: major Milestone: sage-8.1
Component: linear algebra Keywords: sd91
Cc: Merged in:
Authors: Simon Brandhorst Reviewers: Kiran Kedlaya, David Roe
Report Upstream: N/A Work issues:
Branch: 0a0fbe9 (Commits) Commit: 0a0fbe9a30f6fe4c0f1414f5710f1d1900572b5c
Dependencies: #23703 Stopgaps:

Description

sage: FreeModule(ZZ,1)==FreeModule(ZZ,1,inner_product_matrix=matrix.identity(1)*2)
True

Since these two objects are mathematically rather different, this should return False.

Change History (11)

comment:1 Changed 2 years ago by sbrandhorst

  • Component changed from PLEASE CHANGE to linear algebra

comment:2 Changed 2 years ago by sbrandhorst

  • Keywords sd91 added

comment:3 Changed 2 years ago by sbrandhorst

  • Branch set to u/sbrandhorst/include_the_inner_product_matrix_in_module_comparison

comment:4 Changed 2 years ago by git

  • Commit set to b3fa0517a59c360a990a8f6c66d415b882760c92

Branch pushed to git repo; I updated commit sha1. New commits:

b3fa051Moved doctest

comment:5 Changed 2 years ago by sbrandhorst

  • Status changed from new to needs_review

comment:6 Changed 2 years ago by roed

  • Authors set to Simon Brandhorst
  • Reviewers set to David Roe
  • Status changed from needs_review to positive_review

Looks good to me.

comment:7 Changed 2 years ago by kedlaya

  • Status changed from positive_review to needs_work

I'm afraid these doctest failures from patchbot appear to be reproducible:

sage -t --long src/sage/categories/pushout.py  # 2 doctests failed
sage -t --long src/sage/categories/homset.py  # 3 doctests failed

Is it just that these were based on wrong behavior and should be changed?

comment:8 Changed 2 years ago by git

  • Commit changed from b3fa0517a59c360a990a8f6c66d415b882760c92 to 0a0fbe9a30f6fe4c0f1414f5710f1d1900572b5c

Branch pushed to git repo; I updated commit sha1. New commits:

0a0fbe9Adapted _eq_ for sage.categories.Homset and modifided code for pushout where != was used

comment:9 Changed 2 years ago by sbrandhorst

  • Status changed from needs_work to needs_review

Some were based on wrong behavior and some are new bugs. Please check carefully. This is the first time I messed with categores. Not 100% sure I know the consequences. The doctests you mentioned pass now.

comment:10 Changed 2 years ago by kedlaya

  • Reviewers changed from David Roe to Kiran Kedlaya, David Roe
  • Status changed from needs_review to positive_review

Changes look fine to me. All tests pass on k8s. Positive review!

comment:11 Changed 2 years ago by vbraun

  • Branch changed from u/sbrandhorst/include_the_inner_product_matrix_in_module_comparison to 0a0fbe9a30f6fe4c0f1414f5710f1d1900572b5c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.