Opened 10 years ago

Closed 10 years ago

#11549 closed defect (fixed)

Arithmetic with symbolic vectors always creates a new FreeModuleElement_generic_dense

Reported by: jvkersch Owned by: burcin
Priority: major Milestone: sage-4.7.2
Component: symbolics Keywords: vector, symbolic, arithmetic
Cc: roed, robertwb, jason Merged in: sage-4.7.2.alpha0
Authors: Joris Vankerschaver, Jeroen Demeyer Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

Arithmetic with symbolic vectors (addition, scalar multiplication) produces an instance of FreeModuleElement_generic_dense instead of a new symbolic vector:

sage: v = vector(SR, [1])
sage: w = vector(SR, [x])
sage: type(v)
<class 'sage.modules.vector_symbolic_dense.Vector_symbolic_dense'> 
sage: type(w) 
<class 'sage.modules.vector_symbolic_dense.Vector_symbolic_dense'> 
sage: type(v + w)
<type 'sage.modules.free_module_element.FreeModuleElement_generic_dense'>

As David Roe pointed out, the problem is due to the constructor in FreeModuleElement_generic_dense hard-coding the class, so that arithmetic operations with symbolic vectors ends up being a FreeModuleElement_generic_dense rather than an instance of Vector_symbolic_dense.

See the discussion at https://groups.google.com/group/sage-devel/browse_thread/thread/b20559a111041c3b

Apply trac_11549_symbolic_vector_arithmetic.patch and 11549_doc.patch

Attachments (2)

trac_11549_symbolic_vector_arithmetic.patch (2.7 KB) - added by jvkersch 10 years ago.
11549_doc.patch (2.1 KB) - added by jdemeyer 10 years ago.
Small documentation fixes

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by jvkersch

comment:1 Changed 10 years ago by jvkersch

  • Cc jason added
  • Status changed from new to needs_review

The current patch implements the modification in _new_c proposed in sage-devel, and adds an extra doctest to Vector_symbolic_dense. Interestingly, this change also affects the formatting of Vector_callable_symbolic_dense: instead of

sage: f(r, theta, z) = (r*cos(theta), r*sin(theta), z), f
(r, theta, z) |--> (r*cos(theta), r*sin(theta), z)
sage: f+f
((r, theta, z) |--> 2*r*cos(theta), (r, theta, z) |--> 2*r*sin(theta), (r, theta, z) |--> 2*z)
sage: 3*f
((r, theta, z) |--> 3*r*cos(theta), (r, theta, z) |--> 3*r*sin(theta), (r, theta, z) |--> 3*z)

we now have

sage: f(r, theta, z) = (r*cos(theta), r*sin(theta), z), f
(r, theta, z) |--> (r*cos(theta), r*sin(theta), z)
sage: f+f
(r, theta, z) |--> (2*r*cos(theta), 2*r*sin(theta), 2*z)
sage: 3*f
(r, theta, z) |--> (3*r*cos(theta), 3*r*sin(theta), 3*z)

As far as I can tell, this is a purely cosmetic change, so I've taken the liberty of going ahead and adapting the doctests in Vector_callable_symbolic_dense.

comment:2 Changed 10 years ago by jason

Yep, you've fixed a "bug" in callable symbolic vectors. Thanks!

comment:3 Changed 10 years ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_review to positive_review

Looks good; I applied it manually to 4.7.1.alpha4 and all tests pass.

Changed 10 years ago by jdemeyer

Small documentation fixes

comment:4 Changed 10 years ago by jdemeyer

  • Authors set to Joris Vankerschaver, Jeroen Demeyer
  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:5 Changed 10 years ago by jdemeyer

  • Status changed from needs_work to needs_review

My patch needs review.

comment:6 Changed 10 years ago by jvkersch

David, thanks for the positive review of my patch!

Jeroen, I can review your patch but it will take a few days as I'm currently on the road. If anyone else wants to give this the green light, that's fine too. Thanks for noticing these issues.

comment:7 Changed 10 years ago by jvkersch

  • Status changed from needs_review to positive_review

comment:8 Changed 10 years ago by jdemeyer

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