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: sage-9.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:

Status badges

Description (last modified by Matthias Köppe)

As observed on this Ask SageMath question, it does not seem to be possible to iterate over the non-zero 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 Matthias Köppe

Priority: minormajor

comment:2 Changed 3 years ago by Eric Gourgoulhon

Thanks for opening this ticket. I've added a link to it in the "Algebraic part" section of the meta-ticket #18528.

comment:3 Changed 2 years ago by Matthias Köppe

See also: #30309 - Unify free module elements API: methods dict, monomial_coefficients, etc.

comment:4 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:5 Changed 22 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:6 Changed 18 months ago by Matthias Köppe

Cc: Hongli (Bob) Zhao added

comment:7 Changed 18 months ago by Matthias Köppe

@gh-honglizhaobob This is probably the best ticket for starting

comment:8 in reply to:  7 Changed 18 months ago by Hongli (Bob) Zhao

Replying to mkoeppe:

@gh-honglizhaobob 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 Matthias Köppe

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 Hongli (Bob) Zhao

Branch: u/gh-honglizhaobob/tensors_should_have_a_sparse_iterator

comment:11 Changed 17 months ago by git

Commit: bbed88ad016ef4b2c944e28f011ad0d3516deaf7

Branch pushed to git repo; I updated commit sha1. New commits:

bbed88aadded doc for iter

comment:12 Changed 17 months ago by Matthias Köppe

+        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#looping-techniques

comment:13 Changed 17 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:14 Changed 12 months ago by Matthias Köppe

Milestone: sage-9.5sage-9.6

comment:15 Changed 8 months ago by Matthias Köppe

Milestone: sage-9.6sage-9.7

comment:16 Changed 3 months ago by Matthias Köppe

Cc: Travis Scrimshaw added
Description: modified (diff)
Summary: tensors should have a sparse iteratorComponents should have a sparse iterator

comment:17 Changed 3 months ago by Matthias Köppe

Description: modified (diff)

comment:18 Changed 3 months ago by Matthias Köppe

Branch: u/gh-honglizhaobob/tensors_should_have_a_sparse_iteratoru/mkoeppe/tensors_should_have_a_sparse_iterator

comment:19 Changed 3 months ago by Matthias Köppe

Authors: Matthias Koeppe
Commit: bbed88ad016ef4b2c944e28f011ad0d3516deaf7882c5ea994d702bf82764e4f53959d02934ee08b
Status: newneeds_review

New commits:

3862383src/sage/tensor/modules/comp.py: WIP
882c5eaComponents.items: New

comment:20 Changed 3 months ago by git

Commit: 882c5ea994d702bf82764e4f53959d02934ee08b4958211e8186b777705a27a31663967cdbd0e24e

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

4958211Components.items: New

comment:21 Changed 3 months ago by git

Commit: 4958211e8186b777705a27a31663967cdbd0e24e74d94934cad6eaa082f5beec41dd7440c7441a63

Branch pushed to git repo; I updated commit sha1. New commits:

74d9493Matrix.items: New

comment:22 Changed 3 months ago by Matthias Köppe

Description: modified (diff)
Summary: Components should have a sparse iteratorMatrix and Components should have a sparse iterator

comment:23 Changed 3 months ago by Eric Gourgoulhon

Reviewers: Eric Gourgoulhon
Status: needs_reviewpositive_review

Thanks for implementing this. LGTM.

comment:24 Changed 3 months ago by Matthias Köppe

Thanks!

comment:25 Changed 2 months ago by Matthias Köppe

Milestone: sage-9.7sage-9.8

comment:26 Changed 2 months ago by Volker Braun

Branch: u/mkoeppe/tensors_should_have_a_sparse_iterator74d94934cad6eaa082f5beec41dd7440c7441a63
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.