Skip to content

Implicit residual metadata is not registered in the duplicate-name index #79

Description

@chrislupp

Problem

Discipline.add_output appends a second VariableMetaData typed kResidual for implicit disciplines, but never registers it in _declared. The duplicate-name guard therefore covers the output and not its residual.

philote_mdo/general/discipline.py (main, lines 218-227):

self._var_meta += [out_meta]
self._declared.add(key)                       # (kOutput, name) registered

if self._is_implicit:
    res_meta = data.VariableMetaData()
    res_meta.type = data.VariableType.kOutput      # dead: overwritten below
    res_meta.name = name
    if not dynamic_shape:
        res_meta.shape.extend(shape)
    res_meta.units = units
    res_meta.type = data.VariableType.kResidual
    res_meta.dynamic_shape = dynamic_shape
    self._var_meta += [res_meta]                   # not added to _declared

Confirmed on an implicit discipline declaring one output:

_var_meta types: [3, 2]        # kOutput, kResidual
_declared:       {(3, 'x')}    # residual key absent

Consequences

  1. The guard is asymmetric. _declared is the index that makes the duplicate check cheap rather than a quadratic scan (added in Speed up serialization and remove the quadratic metadata scans #74). A residual entry can be created that the guard does not know about, so the two lists can drift.
  2. preallocate_partials resolves shapes against the wrong entry. Both server (discipline_server.py) and client (discipline_client.py) build shapes = {var.name: tuple(var.shape) for var in ..._var_meta}, keyed on name alone. The residual comes later in the list, so it overwrites the output. The shapes are identical today, so this is benign -- but it means the partial-shape lookup for ('x','x') silently resolves against the residual metadata, and it will stop being benign if the two ever diverge.
  3. A dead assignment. res_meta.type is set to kOutput and then immediately to kResidual. Harmless, but it suggests the block was edited without being reread.

Proposed fix

Register the residual key alongside the output, drop the dead assignment, and consider keying the shapes dicts in preallocate_partials and _recover_partials on (type, name) rather than name so the lookup is unambiguous. SetVariableShapes already indexes by (type, name) for exactly this reason (discipline_server.py), so the two sites would then agree.

Notes

Found while tracing the implicit stack for #76. Pre-existing on main.

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions