Allow matrix subdivide to optionally return a copy

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

The .subdivide matrix method acts in place. There are situations when a copy, with the subdivisions, is desirable. An optional keyword, copy will provide this option.

CC: @ThomasGagne

Component: linear algebra

Keywords: matrix subdivide copy

Branch/Commit: u/rbeezer/matrix-copy-on-subdivide @ e974b54

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/18481

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

comment:1

I'd like to have the sage_input() command preserve the subdivisions of matrix. So the output of sage_input(A) could be something like

matrix(QQ, [[1,2,3], [4,5,6]]).subdivide([1], [2], copy=True)

presuming the copy option proposed here was functional. So this ticket is a precursor to the upcoming sage_input ticket. Work on the present ticket is close to finished.

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

Branch: u/rbeezer/matrix-copy-on-subdivide

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

Commit: e974b54

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

New commits:

e974b54

18481: optionally copy matrix when adding subdivisions

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

comment:5

Follow-on ticket, with adjustments to the sage_input() command for matrices, is at #18550.

[videlec]

comment:6

Argh. There is always a choice between inplace and copy argument... we should definitely fix the standard for Sage. A very rough search gives

$ grep "^[[:space:]]*def [a-zA-Z0-9_]*(" $(find . -name "*.py") | grep "copy="
./matrix/matrix_space.py:    def __call__(self, entries=None, coerce=True, copy=True, sparse = False):
./matrix/matrix_space.py:    def matrix(self, x=0, coerce=True, copy=True):
./combinat/combinat.py:    def __init__(self, l, copy=True):
./combinat/designs/group_divisible_designs.py:    def __init__(self, points, groups, blocks, G=None, K=None, lambd=1, check=True, copy=True,**kwds):
./combinat/designs/bibd.py:    def __init__(self, points, blocks, K=None, lambd=1, check=True, copy=True,**kwds):
./combinat/designs/bibd.py:    def __init__(self, points, blocks, k=None, lambd=1, check=True, copy=True,**kwds):
./graphs/generic_graph.py:    def networkx_graph(self, copy=True):
./modules/free_module.py:    def _element_constructor_(self, x, coerce=True, copy=True, check=True):
./geometry/newton_polygon.py:    def vertices(self, copy=True):

against

e$ grep "^[[:space:]]*def [a-zA-Z0-9_]*(" $(find . -name "*.py") | grep "inplace="
./combinat/root_system/dynkin_diagram.py:    def relabel(self, relabelling, inplace=False, **kwds):
./combinat/ordered_tree.py:    def normalize(self, inplace=False):
./combinat/ordered_tree.py:    def dendrog_normalize(self, inplace=False):
./combinat/cluster_algebra_quiver/cluster_seed.py:    def mutate(self, sequence, inplace=True):
./combinat/cluster_algebra_quiver/quiver.py:    def principal_extension(self, inplace=False):
./combinat/cluster_algebra_quiver/quiver.py:    def mutate(self, data, inplace=True):
./combinat/designs/incidence_structures.py:    def relabel(self, perm=None, inplace=True):
./combinat/yang_baxter_graph.py:    def relabel_vertices(self, v, relabel_operator, inplace=True):
./combinat/yang_baxter_graph.py:    def relabel_edges(self, edge_dict, inplace=True):
./combinat/yang_baxter_graph.py:    def relabel_vertices(self, v, inplace=True):
./graphs/digraph.py:    def reverse_edge(self, u, v=None, label=None, inplace=True, multiedges=None):
./graphs/digraph.py:    def reverse_edges(self, edges, inplace=True, multiedges=None):
./graphs/generic_graph.py:    def subgraph(self, vertices=None, edges=None, inplace=False,
./graphs/generic_graph.py:    def _subgraph_by_deleting(self, vertices=None, edges=None, inplace=False,
./graphs/generic_graph.py:    def random_subgraph(self, p, inplace=False):
./graphs/generic_graph.py:    def relabel(self, perm=None, inplace=True, return_map=False, check_input = True, complete_partial_function = True):
./graphs/generic_graph.py:def graph_isom_equivalent_non_edge_labeled_graph(g, partition=None, standard_label=None, return_relabeling=False, return_edge_labels=False, inplace=False, ignore_edge_labels=False):
./modular/arithgroup/arithgroup_perm.py:    def relabel(self, inplace=True):

What do you think? Is it worth opening a thread on sage-devel?

Vincent

[videlec]

Reviewer: Vincent Delecroix

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

comment:7

Dear Vincent,

Thanks for looking at this, and for the excellent grep'ing skills. Sorry I didn't get to this right away.

Well, it looks like inplace wins by frequency. But a discussion on sage-devel might prove interesting. I'm in no hurry. And maybe others can point to the discussion later (or have it go in the developer's guide now).

I'll be at Sage Days next week (arriving a bit late), so that would be a good time to finish this one off if there is a consensus.

Thanks, Rob

[videlec]

comment:8

Hi Rob,

As I actually said on sage-devel, I would like to keep the following semantics for inplace/copy:

• inplace: modify (or not) the object, like l.remove(1) is an inplace modification on a list l;
• copy: whether the created object will hold the argument or make a copy of them, like matrix(ZZ, data=[1,2,3,4], copy=False).

For the purpose of this ticket, this would be an inplace.

Vincent

[1d7ec08f-60ae-4512-91a6-8324c06eab9f]

comment:9

Dear Vincent,

Back from vacation and now at Sage Days 68 for a few days before classes begin.

What do you think of Volker's suggestion that there could be a .subdivided() method, which would return a changed copy? That sounds like a good solution to me.

Rob

[videlec]

comment:10

Replying to @rbeezer:

What do you think of Volker's suggestion that there could be a .subdivided() method, which would return a changed copy? That sounds like a good solution to me.

+1