Ticket #3962 (closed defect: fixed)

Opened 3 months ago

Last modified 2 months ago

[with patch, positive review] Error in converting vector to SR

Reported by: jason Assigned to: was
Priority: major Milestone: sage-3.2
Component: linear algebra Keywords:
Cc:

Description 

sage: a=(QQ^3).subspace([[1,0,1]])
sage: b=a.basis()[0]
sage: b.change_ring(SR)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/grout/sage/devel/sage-main/sage/modules/<ipython console> in <module>()

/home/grout/sage/devel/sage-main/sage/modules/free_module_element.pyx in sage.modules.free_module_element.FreeModuleElement.change_ring (sage/modules/free_module_element.c:3583)()
    407         if P.base_ring() is R:
    408             return self
--> 409         return P.change_ring(R)(self)
    410
    411     def additive_order(self):

/home/grout/sage/local/lib/python2.5/site-packages/sage/modules/free_module.py in change_ring(self, R)
   4406             return M.span_of_basis(B)
   4407         else:
-> 4408             return M.span(B)
   4409
   4410     def coordinate_vector(self, v, check=True):

/home/grout/sage/local/lib/python2.5/site-packages/sage/modules/free_module.py in span(self, gens, base_ring, check, already_echelonized)
   2571         if base_ring is None or base_ring == self.base_ring():
   2572             return FreeModule_submodule_field(
-> 2573                 self.ambient_module(), gens=gens, check=check, already_echelonized=already_echelonized)
   2574         else:
   2575             try:

/home/grout/sage/local/lib/python2.5/site-packages/sage/modules/free_module.py in __init__(self, ambient, gens, check, already_echelonized)
   4857             raise TypeError, "Argument gens (= %s) must be a list, tuple, or sequence."%gens
   4858         FreeModule_submodule_with_basis_field.__init__(self, ambient, basis=gens, check=check,
-> 4859             echelonize=not already_echelonized, already_echelonized=already_echelonized)
   4860
   4861     def _repr_(self):

/home/grout/sage/local/lib/python2.5/site-packages/sage/modules/free_module.py in __init__(self, ambient, basis, check, echelonize, echelonized_basis, already_echelonized)
   4683         FreeModule_submodule_with_basis_pid.__init__(
   4684             self, ambient, basis=basis, check=check, echelonize=echelonize,
-> 4685             echelonized_basis=echelonized_basis, already_echelonized=already_echelonized)
   4686
   4687     def _repr_(self):

/home/grout/sage/local/lib/python2.5/site-packages/sage/modules/free_module.py in __init__(self, ambient, basis, check, echelonize, echelonized_basis, already_echelonized)
   3745
   3746         if echelonize and not already_echelonized:
-> 3747             basis = self._echelonized_basis(ambient, basis)
   3748
   3749         FreeModule_generic.__init__(self, R, rank=len(basis), degree=ambient.degree(), sparse=ambient.is_sparse())

/home/grout/sage/local/lib/python2.5/site-packages/sage/modules/free_module.py in _echelonized_basis(self, ambient, basis)
   4794         E = A.echelon_form()
   4795         # Return the first rank rows (i.e., the nonzero rows).
-> 4796         return E.rows()[:E.rank()]
   4797
   4798     def is_ambient(self):

TypeError: slice indices must be integers or None or have an __index__ method

Attached patch fixes this problem

Attachments

trac_3962-3.patch (2.3 kB) - added by jason on 10/01/2008 08:44:15 PM.

Change History

08/26/2008 02:09:27 PM changed by jason

  • summary changed from Error in converting vector to SR to [with patch, needs review] Error in converting vector to SR.

08/28/2008 06:28:22 PM changed by mhansen

I've attached a patch which replaces the first one and fixes the problem by adding an index method for SymbolicConstants?.

08/28/2008 09:30:28 PM changed by jason

  • summary changed from [with patch, needs review] Error in converting vector to SR to [with patch, positive review] Error in converting vector to SR.

mhansen's patch does indeed correct the issue and is much more general, so positive review for his patch. Doctests in the freemodule directory pass.

08/28/2008 11:28:47 PM changed by mabshoff

  • summary changed from [with patch, positive review] Error in converting vector to SR to [with patch, needs work] Error in converting vector to SR.

This patch causes a doctest failure: 

mabshoff@sage:/scratch/mabshoff/release-cycle/sage-3.1.2.alpha2$ ./sage -t -long devel/sage/sage/structure/element.pyx
sage -t -long devel/sage/sage/structure/element.pyx         
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.1.2.alpha2/tmp/element.py", line 1458:
    sage: 2r**(SR(2)-1-1r)
Expected:
    Traceback (most recent call last):
    ...
    NotImplementedError: non-integral exponents not supported
Got:
    1
**********************************************************************
1 items had failures:
   1 of  15 in __main__.example_60
***Test Failed*** 1 failures.
For whitespace errors, see the file /scratch/mabshoff/release-cycle/sage-3.1.2.alpha2/tmp/.doctest_element.py
         [6.1 s]
exit code: 1024

10/01/2008 07:48:52 PM changed by jason

huh, this means that something that used to not work (but should have) now works, so a doctest passes where it used to give an error.

I'd say this is a sign of the success of this patch. Change the doctest so that the result is what it should be!

10/01/2008 07:49:48 PM changed by jason

  • summary changed from [with patch, needs work] Error in converting vector to SR to [with patch, positive review] Error in converting vector to SR.

With the doctest fixed, giving this a positive review.

(follow-up: ↓ 8 ) 10/01/2008 07:50:26 PM changed by jason

So the last two patches should be applied. I haven't tested the last patch, but it's a simple doctest fix, so what could go wrong? :)

(in reply to: ↑ 7 ) 10/01/2008 07:59:10 PM changed by mabshoff

  • summary changed from [with patch, positive review] Error in converting vector to SR to [with patch, needs review] Error in converting vector to SR.

Replying to jason:

So the last two patches should be applied. I haven't tested the last patch, but it's a simple doctest fix, so what could go wrong? :)

Mike Hansen had some reservations, so let's wait until he signs off on it.

Cheers,

Michael

10/01/2008 08:32:53 PM changed by jason

  • summary changed from [with patch, needs review] Error in converting vector to SR to [with patch, needs work] Error in converting vector to SR.

Okay, there is a problem. Since index just converts self to an int, it rounds 1/2 to zero, etc. So we get 2(SR(1/2)) = 1.

Changing the index method to return int(Integer(self)) fixes the problem.

10/01/2008 08:44:15 PM changed by jason

  • attachment trac_3962-3.patch added.

10/01/2008 08:45:46 PM changed by jason

  • summary changed from [with patch, needs work] Error in converting vector to SR to [with patch, needs review] Error in converting vector to SR.

Apply trac_3962-3.patch only. It incorporates both an updated index method and the necessary doctest fixes and additions.

10/14/2008 10:50:16 AM changed by robertwb

  • summary changed from [with patch, needs review] Error in converting vector to SR to [with patch, positive review] Error in converting vector to SR.

Adding the __index__ method is the right thing to do. Works well and applies cleanly to 3.1.3.

10/18/2008 08:11:05 AM changed by mabshoff

  • status changed from new to closed.
  • resolution set to fixed.

Merged trac_3962-3.patch in Sage 3.2.alpha0