Opened 9 years ago

Closed 9 years ago

#14558 closed defect (fixed)

apply_map on sparse vectors returns vectors of smaller degree

Reported by: tfeulner Owned by: AlexGhitza
Priority: major Milestone: sage-5.12
Component: linear algebra Keywords: apply_map, sparse vector
Cc: rbeezer Merged in: sage-5.12.beta3
Authors: Thomas Feulner Reviewers: Rob Beezer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by tfeulner)

Calling apply_map on a sparse vector with zeros at the last positions returns a vector of wrong degree.

sage: F.<a> = GF(9)
sage: v = vector([a, 0,0,0], sparse=True)
sage: f = F.hom([a**3])
sage: v.apply_map(f)
(2*a + 1)

Attachments (1)

trac_14558_apply_map_on_sparse_vectors.patch (1.8 KB) - added by tfeulner 9 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 9 years ago by tfeulner

  • Component changed from algebra to linear algebra
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by rbeezer

  • Cc rbeezer added

comment:3 follow-up: Changed 9 years ago by rbeezer

It looks like you have duplicated some code for finding the common ring. I think that ideally this step should remain in the vector constructor.

Before returning, you have the mapped elements, the degree, and the sparseness. Did you try calling vector() with all the arguments supplied named, ie

arg0, arg1, arg2, sparse

much as calling format number 4, but with sparseness supplied? Or maybe calling format 4 needs to be sure to handle the sparse keyword properly.

I could be wrong about this, since I have not fully tested everything, but is there a way to just do the mapping and pass the degree in the sparse and non-sparse cases? Let the constructor determine the new ring for the mapped entries?

Changed 9 years ago by tfeulner

comment:4 in reply to: ↑ 3 Changed 9 years ago by tfeulner

Replying to rbeezer:

It looks like you have duplicated some code for finding the common ring. I think that ideally this step should remain in the vector constructor.

I agree with you, this should stay in the constructor.

The updated patch now just explicitly adds, in the cases where it is necessary, one zero entry to the dictionary.

comment:5 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:6 Changed 9 years ago by rbeezer

  • Reviewers set to Rob Beezer
  • Status changed from needs_review to positive_review

Sorry to have lost track of this one.

Fix looks good, behaves as expected, passes tests, and docs look good. Positive review.

Thanks for sticking with this one.

Rob

comment:7 Changed 9 years ago by jdemeyer

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