Opened 3 years ago
Closed 2 months ago
#29619 closed enhancement (fixed)
Matrix and Components should have a sparse iterator
Reported by:  Markus Wageringel  Owned by:  

Priority:  major  Milestone:  sage9.8 
Component:  linear algebra  Keywords:  
Cc:  Eric Gourgoulhon, Léo Brunswic, Hongli (Bob) Zhao, Travis Scrimshaw  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Eric Gourgoulhon 
Report Upstream:  N/A  Work issues:  
Branch:  74d9493 (Commits, GitHub, GitLab)  Commit:  74d94934cad6eaa082f5beec41dd7440c7441a63 
Dependencies:  Stopgaps: 
Description (last modified by )
As observed on this Ask SageMath question, it does not seem to be possible to iterate over the nonzero entries of a tensor. This is unfortunate because the entries are stored in a sparse format, in a dictionary.
Since a tensor might have symmetries, this is more involved than just iterating over the dictionary, but such an iterator would immediately be useful for the implementation of the display()
method, for instance.
Here we implement a method Components.items
, returning an iterable of (indices, value)
. This is compatible with the sparse and dense vectors from sage.modules
.
(Because a Components
does not currently have a parent, it does not make sense to define the method monomial_coefficients
 as defined by ModulesWithBasis
.)
We also define Matrix.items
with the same interface.
Change History (26)
comment:1 Changed 3 years ago by
Priority:  minor → major 

comment:2 Changed 3 years ago by
comment:3 Changed 2 years ago by
See also: #30309  Unify free module elements API: methods dict
, monomial_coefficients
, etc.
comment:4 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:5 Changed 22 months ago by
Milestone:  sage9.3 → sage9.4 

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:6 Changed 18 months ago by
Cc:  Hongli (Bob) Zhao added 

comment:7 followup: 8 Changed 18 months ago by
@ghhonglizhaobob This is probably the best ticket for starting
comment:8 Changed 18 months ago by
Replying to mkoeppe:
@ghhonglizhaobob This is probably the best ticket for starting
Is there a set of unit test cases one needs to pass to ensure a justified change of data structure?
comment:9 Changed 18 months ago by
To clarify  here on this ticket, the data structure shouldn't be changed but instead methods added.
Sage mainly uses doctests  see https://doc.sagemath.org/html/en/developer/doctesting.html
but there are also unit tests, which are invoked within the doctests by TestSuite(...).run()
.
comment:10 Changed 17 months ago by
Branch:  → u/ghhonglizhaobob/tensors_should_have_a_sparse_iterator 

comment:11 Changed 17 months ago by
Commit:  → bbed88ad016ef4b2c944e28f011ad0d3516deaf7 

Branch pushed to git repo; I updated commit sha1. New commits:
bbed88a  added doc for iter

comment:12 Changed 17 months ago by
+ for multi_idx in self._comp.keys(): + # access value in dictionary + coef = self._comp[multi_idx]
You can use .items()
here  see https://docs.python.org/3/tutorial/datastructures.html#loopingtechniques
comment:13 Changed 17 months ago by
Milestone:  sage9.4 → sage9.5 

comment:14 Changed 12 months ago by
Milestone:  sage9.5 → sage9.6 

comment:15 Changed 8 months ago by
Milestone:  sage9.6 → sage9.7 

comment:16 Changed 3 months ago by
Cc:  Travis Scrimshaw added 

Description:  modified (diff) 
Summary:  tensors should have a sparse iterator → Components should have a sparse iterator 
comment:17 Changed 3 months ago by
Description:  modified (diff) 

comment:18 Changed 3 months ago by
Branch:  u/ghhonglizhaobob/tensors_should_have_a_sparse_iterator → u/mkoeppe/tensors_should_have_a_sparse_iterator 

comment:19 Changed 3 months ago by
Authors:  → Matthias Koeppe 

Commit:  bbed88ad016ef4b2c944e28f011ad0d3516deaf7 → 882c5ea994d702bf82764e4f53959d02934ee08b 
Status:  new → needs_review 
comment:20 Changed 3 months ago by
Commit:  882c5ea994d702bf82764e4f53959d02934ee08b → 4958211e8186b777705a27a31663967cdbd0e24e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4958211  Components.items: New

comment:21 Changed 3 months ago by
Commit:  4958211e8186b777705a27a31663967cdbd0e24e → 74d94934cad6eaa082f5beec41dd7440c7441a63 

Branch pushed to git repo; I updated commit sha1. New commits:
74d9493  Matrix.items: New

comment:22 Changed 3 months ago by
Description:  modified (diff) 

Summary:  Components should have a sparse iterator → Matrix and Components should have a sparse iterator 
comment:23 Changed 3 months ago by
Reviewers:  → Eric Gourgoulhon 

Status:  needs_review → positive_review 
Thanks for implementing this. LGTM.
comment:25 Changed 2 months ago by
Milestone:  sage9.7 → sage9.8 

comment:26 Changed 2 months ago by
Branch:  u/mkoeppe/tensors_should_have_a_sparse_iterator → 74d94934cad6eaa082f5beec41dd7440c7441a63 

Resolution:  → fixed 
Status:  positive_review → closed 
Thanks for opening this ticket. I've added a link to it in the "Algebraic part" section of the metaticket #18528.