Ticket #2461 (closed defect: fixed)

Opened 5 years ago

Last modified 5 years ago

[with patch, with positive review] vector norms should have a reasonable default

Reported by: robertwb Owned by: somebody
Priority: major Milestone: sage-2.10.4
Component: basic arithmetic Keywords:
Cc: Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description

v.norm() should work without any arguments, returning the (standard) Euclidean norm.

Attachments

vector-norms.patch Download (1.7 KB) - added by robertwb 5 years ago.
vector-norms_replace.patch Download (2.5 KB) - added by AlexGhitza 5 years ago.

Change History

Changed 5 years ago by robertwb

comment:1 Changed 5 years ago by robertwb

  • Summary changed from vector norms should have a reasonable default to [with patch, needs review] vector norms should have a reasonable default

comment:2 Changed 5 years ago by AlexGhitza

  • Summary changed from [with patch, needs review] vector norms should have a reasonable default to [with new patch, needs review] vector norms should have a reasonable default

I was in the process of refereeing this, and realized that Robert's changes to the doctests uncovered a fairly serious bug in the code for norm(): 

sage: v = vector([1, 2, -3])
sage: v.norm(Infinity)
2
sage: v.norm(1)
0

Both of these are wrong, due to the fact that whoever wrote the norm() function (I think it was me, actually) forgot to take absolute values of the entries of the vector before computing the norm.

I fixed this and put up a new patch that incorporates both this fix and Robert's improvements.

Changed 5 years ago by AlexGhitza

comment:3 Changed 5 years ago by rlm

  • Milestone changed from sage-2.11 to sage-2.10.4

comment:4 Changed 5 years ago by cwitty

  • Summary changed from [with new patch, needs review] vector norms should have a reasonable default to [with patch, with positive review] vector norms should have a reasonable default

Looks good, testall passes.

Apply vector-norms_replace.patch; when writing release notes, note that this patch combines work by robertwb and AlexGhitza.

comment:5 Changed 5 years ago by mabshoff

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

Merged in Sage 2.10.4.alpha0

Note: See TracTickets for help on using tickets.