Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9502 closed defect (fixed)

Basis parent bug in FreeModule

Reported by: novoselt Owned by: AlexGhitza
Priority: major Milestone: sage-4.6
Component: linear algebra Keywords:
Cc: vbraun Merged in: sage-4.6.alpha1
Authors: Andrey Novoseltsev Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by novoselt)

There is an inconsistency in the example below for the echelonized basis of submodules with basis:

sage: F = FreeModule(ZZ, 3)
sage: S = F.submodule_with_basis([(1,2,3),(3,2,1)])
sage: parent(S.basis()[0])
Free module of degree 3 and rank 2 over Integer Ring
User basis matrix:
[1 2 3]
[3 2 1]
sage: parent(S.echelonized_basis()[0])
Ambient free module of rank 3 over the principal ideal domain Integer Ring

For automatic bases everything is OK:

sage: S = F.submodule([(1,2,3),(3,2,1)])
sage: parent(S.echelonized_basis()[0])
Free module of degree 3 and rank 2 over Integer Ring
Echelon basis matrix:
[1 2 3]
[0 4 8]
sage: parent(S.basis()[0])
Free module of degree 3 and rank 2 over Integer Ring
Echelon basis matrix:
[1 2 3]
[0 4 8]

While I was working on this patch, I expanded documentation for FreeModule_submodule_with_basis_pid and rewrote its constructor to fix the issue on this ticket and make its logic more clear.

I have also discovered that it avoids calling the base constructor which checks that the base ring is indeed a PID. I tried to fix, got errors, fixed one of them (thanks to Mike Hansen), but there are more with number fields and since it was not the main issue on this ticket I delegated it to #9503.

Current patch passed all tests on 4.5.alpha1.

Attachments (2)

trac_9502_basis_parent_bug_in_FreeModule.patch (9.3 KB) - added by novoselt 11 years ago.
trac_9502_basis_parent_bug_in_FreeModule.2.patch (8.4 KB) - added by novoselt 11 years ago.
Reverted change to symbolic/ring.pyx

Download all attachments as: .zip

Change History (14)

comment:1 Changed 11 years ago by novoselt

  • Authors set to Andrey Novoseltsev
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 11 years ago by vbraun

  • Cc vbraun added

comment:3 Changed 11 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Fixes the original issue and is a general improvement! Tests OK on Sage 4.5, too.

comment:4 follow-up: Changed 11 years ago by ddrake

  • Status changed from positive_review to needs_work

I applied the patch to 4.5.2.alpha0 and when I try to start Sage, I get "ImportError?: cannot import name SR". Any ideas? I've rebuilt the entire Sage library and the problem persists.

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

Replying to ddrake:

I applied the patch to 4.5.2.alpha0 and when I try to start Sage, I get "ImportError?: cannot import name SR". Any ideas? I've rebuilt the entire Sage library and the problem persists.

That's strange, since the patch does not mention SR...

comment:6 in reply to: ↑ 5 Changed 11 years ago by kcrisman

Replying to novoselt:

Replying to ddrake:

I applied the patch to 4.5.2.alpha0 and when I try to start Sage, I get "ImportError?: cannot import name SR". Any ideas? I've rebuilt the entire Sage library and the problem persists.

That's strange, since the patch does not mention SR...

Sure it does, implicitly - the final bit is part of

cdef class SymbolicRing(CommutativeRing):

which is changed. And #8562 in alpha0 did change the behavior of Fields, though it's not clear to me how that change would affect SR's ability to import...

Changed 11 years ago by novoselt

Reverted change to symbolic/ring.pyx

comment:7 Changed 11 years ago by novoselt

  • Status changed from needs_work to needs_review

I have reverted the change to symbolic/ring.pyx since it is actually not necessary for this ticket (it may be relevant for #9503). Now I don't have problems with 4.5.2.alpha0.

comment:8 Changed 11 years ago by vbraun

  • Status changed from needs_review to positive_review

Works on 4.5.2. Positive review!

For the maintainer: Apply trac_9502_basis_parent_bug_in_FreeModule.2.patch only.

comment:9 Changed 11 years ago by AlexGhitza

  • Component changed from algebra to linear algebra

comment:10 Changed 11 years ago by mpatel

  • Merged in set to sage-4.6.alpha1

comment:11 Changed 11 years ago by mpatel

  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:12 Changed 11 years ago by ddrake

For future reference: this ticket might have introduced a segfault; see http://groups.google.com/group/sage-devel/t/b1f3a7bec3ba655f and #10250.

Note: See TracTickets for help on using tickets.