Skip to content

Network.initialize(): UnboundLocalError when the Network was constructed with segments #24

Description

@awickert

Network.initialize() raises UnboundLocalError whenever the Network was constructed with a list of segments, because segments is bound only inside the branch that builds them.

Reproduction

Against master @ 9724052:

import numpy as np, grlp

seg = grlp.Segment()
net = grlp.Network(segments=[seg])
net.initialize(
    upstream_segment_IDs=[[]], downstream_segment_IDs=[[]],
    x=[np.arange(0., 1000., 100.)], z=[np.linspace(10., 0., 10)],
    Q=[np.ones(10)], B=[np.ones(10)], x_bl=1000., z_bl=0.)
  File "grlp/grlp.py", line 1493, in initialize
    self.segments = segments
                    ^^^^^^^^
UnboundLocalError: cannot access local variable 'segments' where it is not associated with a value

Mechanism

Network.__init__ — L979 accepts segments, and its docstring advertises this as the way to build a network ("Instantiate the Network object with a list of Long Profile objects"). initialize() then defaults to overwrite=False — L1427, so with segments already present it takes the not-rebuilding branch:

_build_segments = True                     # L1469
if self.segments is not None:
    if overwrite:
        print("Overwriting prior network segments.")
    else:
        _build_segments = False            # L1474
...
if _build_segments:                        # L1487
    nseg = len(x)
    segments = []                          # the ONLY binding of `segments`
    for i in range(nseg):
        segments.append( Segment() )
# Class var; clunkier name
self.segments = segments                   # L1493 -- nothing bound on the else path

L1469-L1493

Why nothing catches it

Every test constructs grlp.Network() with no arguments, so the branch is never taken. The ten scripts under examples/deprecated/ that do use grlp.Network(segments) configure the segments by hand and go straight to evolve_threshold_width_river_network() — none of them calls initialize() afterwards. So no shipped code exercises the combination of the segments-passing constructor and initialize(), which is the pair that crashes.

Suggested fix

Bind the existing segments on the else path:

if _build_segments:
    nseg = len(x)
    segments = []
    for i in range(nseg):
        segments.append( Segment() )
else:
    # Not overwriting: reuse the Segment objects already on the Network
    # (passed to __init__). They are re-populated from the arguments below,
    # so "overwrite" governs the objects, not their contents.
    segments = self.segments
# Class var; clunkier name
self.segments = segments

That reading of overwrite — it governs whether the Segment objects are rebuilt, not whether their contents are re-populated — is what makes the rest of initialize() coherent, since the loop below re-applies x, z, Q and B to each segment either way. Worth confirming that is the intent.

One open question the fix does not settle: if len(self.segments) != len(x), the loop below will IndexError. Whether to validate that, and with what message, is a design call rather than part of this bug.

How this was found

Running pyright over this engine — it reports segments as possibly-unbound at L1493 and L1496. Related but separate: #23.

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions