Revision history [back]

When you copy a list of lists, the inside lists are not copies.

Example:

sage: a = [[0, 0], [0, 0], [0, 0]]
sage: b = a.copy()
sage: b[0][0] = 1
sage: b
[[1, 0], [0, 0], [0, 0]]
sage: a
[[1, 0], [0, 0], [0, 0]]

Use deepcopy to copy nested lists.

sage: c =  [[0, 0], [0, 0], [0, 0]]
sage: d = deepcopy(c)
sage: d[0][0] = 1
sage: d
[[1, 0], [0, 0], [0, 0]]
sage: c
[[0, 0], [0, 0], [0, 0]]

When you copy The thing is that when copying a list of lists, lists, the inside lists are not copies.

Example:copied but end up being the same lists as in the original list.

The workaround is to use deepcopy which copies nested lists recursively.

Here is a simpler illustration of that.

The problem with copying nested lists with copy:

sage: a = [[0, 0], [0, 0], [0, 0]]
sage: b = a.copy()
sage: b[0][0] = 1
sage: b
[[1, 0], [0, 0], [0, 0]]
sage: a
[[1, 0], [0, 0], [0, 0]]

Use How deepcopy to copy nested lists.avoids this problem:

sage: c =  [[0, 0], [0, 0], [0, 0]]
sage: d = deepcopy(c)
sage: d[0][0] = 1
sage: d
[[1, 0], [0, 0], [0, 0]]
sage: c
[[0, 0], [0, 0], [0, 0]]

def edge_on_faces(j, S, Gf, G1, Gd, Gt):
    r"""
    Return whether ...
    """
    variables = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
        return (0 <= j < len(variables)
                and any(str(e[2]) == variables[j]
                        for n in S for e in Gf[n]))

def face_modification(i, Gf, G1, Gd, Gt):
    r"""
    Return ...
    """
    variables = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
    l = [m for m in range(len(Gf)) if edge_on_faces(i, [m], Gf, G1, Gd, Gt)]
    # PF: list of two faces in Gf with the ith edge on the boundary
    PF = [deepcopy(Gf)[e] for e in l]
    GGf = deepcopy(Gf)
    for p in PF:
        GGf.remove(p)
        for k in range(len(p)):
            if str(p[k][2]) == variables[i]:
                p.remove(p[k])
    if len(PF) == 2:
        B = GGf + [PF[0] + PF[1]]
        return B
    else:
        raise ValueError('unexpected situation in face_modification')

The thing is that when copying a list of lists, the inside lists are not copied but end up being the same lists as in the original list.

The workaround is to use deepcopy which copies nested lists recursively.

Here is a simpler illustration of that.

The problem with copying nested lists with copy:

sage: a = [[0, 0], [0, 0], [0, 0]]
sage: b = a.copy()
sage: b[0][0] = 1
sage: b
[[1, 0], [0, 0], [0, 0]]
sage: a
[[1, 0], [0, 0], [0, 0]]

How deepcopy avoids this problem:

sage: c =  [[0, 0], [0, 0], [0, 0]]
sage: d = deepcopy(c)
sage: d[0][0] = 1
sage: d
[[1, 0], [0, 0], [0, 0]]
sage: c
[[0, 0], [0, 0], [0, 0]]

Besides that, the code could be streamlined by iterating by values instead of by indices.

Suggestion: go through some of the tutorial at

• https://doc.sagemath.org/html/en/thematic_tutorials

in particular

• Programming in Python and Sage
• Comprehensions, iterators and iterables

Here is an attempt at making the code from the question more concise.

In this rewrite,

• edge_on_faces returns booleans rather than integers
• iterating by values insted of by index makes loop simpler
• variablesvars are in string form since they are only used as such
• double comprehension avoids summing list when defining vars
• any avoids building the whole list if ot necessary
• raising an exception replaces returning the string 'error'

Here we go. All that is missing is to fill in the documentation string for each function.

def edge_on_faces(j, S, Gf, G1, Gd, Gt):
    r"""
    Return whether ...
    """
    variables vars = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
     return (0 <= j < len(variables)
    len(vars)
            and any(str(e[2]) == variables[j]
                        vars[j] for n in S for e in Gf[n]))

def face_modification(i, Gf, G1, Gd, Gt):
    r"""
    Return ...
    """
    variables vars = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
    l = [m for m in range(len(Gf)) if edge_on_faces(i, [m], Gf, G1, Gd, Gt)]
    # PF: list of two faces in Gf with the ith edge on the boundary
    PF = [deepcopy(Gf)[e] for e in l]
    GGf = deepcopy(Gf)
    for p in PF:
        GGf.remove(p)
        for k in range(len(p)):
            if str(p[k][2]) == variables[i]:
vars[i]:
                p.remove(p[k])
    if len(PF) == 2:
        B = GGf + [PF[0] + PF[1]]
        return B
    else:
        raise ValueError('unexpected situation in face_modification')

The thing is that when copying a list of lists, the inside lists are not copied but end up being the same lists as in the original list.

The workaround is to use deepcopy which copies nested lists recursively.

Here is a simpler illustration of that.

The problem with copying nested lists with copy:

sage: a = [[0, 0], [0, 0], [0, 0]]
sage: b = a.copy()
sage: b[0][0] = 1
sage: b
[[1, 0], [0, 0], [0, 0]]
sage: a
[[1, 0], [0, 0], [0, 0]]

How deepcopy avoids this problem:

sage: c =  [[0, 0], [0, 0], [0, 0]]
sage: d = deepcopy(c)
sage: d[0][0] = 1
sage: d
[[1, 0], [0, 0], [0, 0]]
sage: c
[[0, 0], [0, 0], [0, 0]]

Besides that, the code could be streamlined by iterating by values instead of by indices.

Suggestion: go through some of the tutorial at

• https://doc.sagemath.org/html/en/thematic_tutorials

in particular

• Programming in Python and Sage
• Comprehensions, iterators and iterables

Here is an attempt at making the code from the question more concise.

In this rewrite,

• edge_on_faces returns booleans rather than integers
• iterating by values insted of by index makes loop simpler
• vars are in string form since they are only used as such
• double comprehension avoids summing list when defining vars
• any avoids building the whole list if ot necessary
• raising an exception replaces returning the string 'error'
• copying Gf is done only once

Here we go. All that is missing is to fill in the documentation string for each function.

def edge_on_faces(j, S, Gf, G1, Gd, Gt):
    r"""
    Return whether ...
    """
    vars = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
    return (0 <= j < len(vars)
            and any(str(e[2]) == vars[j] for n in S for e in Gf[n]))

def face_modification(i, Gf, G1, Gd, Gt):
    r"""
    Return ...
    """
    vars = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
    l = [m for m in range(len(Gf)) if edge_on_faces(i, [m], Gf, G1, Gd, Gt)]
    # PF: list of two faces in Gf with the ith edge on the boundary
    GGf = deepcopy(Gf)
    PF = [deepcopy(Gf)[e] [GGf[e] for e in l]
    GGf = deepcopy(Gf)
    for p in PF:
        GGf.remove(p)
        for k in range(len(p)):
            if str(p[k][2]) == vars[i]:
                p.remove(p[k])
    if len(PF) == 2:
        B = GGf + [PF[0] + PF[1]]
        return B
    else:
        raise ValueError('unexpected situation in face_modification')

The changes should also make the code faster.

There might be ways to simplify further.

The thing is that when copying a list of lists, the inside lists are not copied but end up being the same lists as in the original list.

The workaround is to use deepcopy which recursively copies nested lists recursively.lists.

Here is a simpler illustration of that.

The problem with copying nested lists with copy:

sage: a = [[0, 0], [0, 0], [0, 0]]
sage: b = a.copy()
sage: b[0][0] = 1
sage: b
[[1, 0], [0, 0], [0, 0]]
sage: a
[[1, 0], [0, 0], [0, 0]]

How deepcopy avoids this problem:

sage: c =  [[0, 0], [0, 0], [0, 0]]
sage: d = deepcopy(c)
sage: d[0][0] = 1
sage: d
[[1, 0], [0, 0], [0, 0]]
sage: c
[[0, 0], [0, 0], [0, 0]]

Besides that, the code could be streamlined by iterating by values instead of by indices.

Suggestion: go through some of the tutorial at

• https://doc.sagemath.org/html/en/thematic_tutorials

in particular

• Programming in Python and Sage
• Comprehensions, iterators and iterables

Here is an attempt at making the code from the question more concise.

In this rewrite,

• edge_on_faces returns booleans rather than integers
• iterating by values insted of by index makes loop simpler
• vars are in string form since they are only used as such
• double comprehension avoids summing list when defining vars
• any avoids building the whole list if ot necessary
• raising an exception replaces returning the string 'error'
• copying Gf is done only once
• avoid summing lists and assigning to B; instead append to GGf and return it

Here we go. All that is missing is to fill in the documentation string for each function.

def edge_on_faces(j, S, Gf, G1, Gd, Gt):
    r"""
    Return whether ...
    """
    vars = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
    return (0 <= j < len(vars)
            and any(str(e[2]) == vars[j] for n in S for e in Gf[n]))

def face_modification(i, Gf, G1, Gd, Gt):
    r"""
    Return ...
    """
    vars = [str(e[2]) for G in (G1, Gd, Gt) for e in G.edges()]
    l = [m for m in range(len(Gf)) if edge_on_faces(i, [m], Gf, G1, Gd, Gt)]
    # PF: list of two faces two-faces in Gf with the ith that have edge i on the their boundary
    GGf = deepcopy(Gf)
    PF = [GGf[e] for e in l]
    for p in PF:
        GGf.remove(p)
        for k in range(len(p)):
            if str(p[k][2]) == vars[i]:
                p.remove(p[k])
    if len(PF) == 2:
        B = GGf GGf.append(PF[0] + [PF[0] + PF[1]]
PF[1])
        return B
GGf
    else:
        raise ValueError('unexpected situation in face_modification')

The changes should also make the code faster.

There might be ways to simplify further.

        for k in range(len(p)):
            if str(p[k][2]) == vars[i]:
                p.remove(p[k])