A cancelled future raises AttributeError instead of the intended PybnfError when smoothing or parallelize_models is on

[wshlavacek]

What happens

result_from_completed deliberately returns the raw CancelledError object for a cancelled future (core.py:578-579, documented as 'returned unchanged for the caller to treat as fatal'), and _record_result_and_decide is the place that treats it as fatal -- base.py:1243-1245 turns it into a PybnfError telling the user to restart with -r.

But the run loop folds group results before it ever reaches that check:

1586  if self.config.config['smoothing'] > 1 or self.config.config['parallelize_models'] > 1:
1587      res = self._fold_group_result(res)

and _fold_group_result opens with group = self.job_group_dir.pop(res.name) (base.py:1211). concurrent.futures.CancelledError has no .name, so on any fit with smoothing > 1 or parallelize_models > 1 a cancelled future raises AttributeError: 'CancelledError' object has no attribute 'name' out of the run loop instead of the user-facing message -- and it escapes before run() reaches client.cancel(...) / _finalize_run(client) at base.py:1358-1360, so the remaining futures are never cancelled and the end-of-fit artifacts (final parameter table, best-fit re-simulation, stop_reason.txt) are never written for work that was already paid for.

The asymmetry is what makes this clearly unintended rather than a design choice: the two other collection loops in this same file guard correctly before folding -- _run_confirmation_replicates checks if not isinstance(res, core.Result): ... continue at base.py:1821-1824 and _run_information_criteria_jobs checks the same at base.py:2194-2199. Only the primary run loop folds first and type-checks second. (A related consequence of the same missing guard: were .name present, job_group_dir.pop with no default would raise KeyError.)

Reproduction

Ran this against the real class in the project env (read-only, scratchpad only):

cd /Users/l119605/Code/PyBNF && uv run python /private/tmp/claude-503/-Users-l119605-Code-PyBNF/eb55c5a3-1006-4c5d-9d1e-9d14fe8045f0/scratchpad/r2.py

r2.py:
import sys; sys.path.insert(0, '/Users/l119605/Code/PyBNF')
from pybnf import algorithms
from pybnf.algorithms.base import Algorithm
class A(Algorithm):
def start_run(self): return []
def got_result(self, res): return []
alg = object.new(A)
alg.job_group_dir = {}
try:
alg._fold_group_result(algorithms.CancelledError('sim_1'))
except Exception as e:
print(type(e).name, ':', e)

Observed: AttributeError : 'CancelledError' object has no attribute 'name'
Expected (what the same input gets at smoothing=1, via _record_result_and_decide): PybnfError('PyBNF has encountered a fatal error... To resume the run please restart PyBNF using the -r flag').

End-to-end shape (not executed, needs a cluster): any conf with smoothing=3 (or parallelize_models=2); kill a worker mid-generation so dask returns one future cancelled. The user sees "Sorry, an unknown error occurred: AttributeError: 'CancelledError' object has no attribute 'name'" (pybnf/pybnf.py:771-777) instead of the -r restart guidance (pybnf/pybnf.py:761-766).

Verification notes

The claim survives, but its consequences are overstated; I corrected severity down to low.

Confirmed mechanics:

• /Users/l119605/Code/PyBNF/pybnf/algorithms/core.py:578-579 — result_from_completed returns a concurrent.futures.CancelledError object unchanged (pinned by tests/test_failed_sim_handling.py:70-75 test_cancelled_future_returned_unchanged), so a non-Result really can reach the run loop.
• /Users/l119605/Code/PyBNF/pybnf/algorithms/base.py:1586-1589 (_drain_job_pool) folds first and type-checks second: if smoothing > 1 or parallelize_models > 1: res = self._fold_group_result(res), and only the later _record_result_and_decide (base.py:1243-1245) has the isinstance(res, CancelledError) guard.
• /Users/l119605/Code/PyBNF/pybnf/algorithms/base.py:1211 — group = self.job_group_dir.pop(res.name). job_group_dir is a plain dict() (base.py:194, 299), not a defaultdict, and CancelledError has no .name (it is a plain Exception subclass).
• The asymmetry the claimant cites is real: base.py:1818-1824 (_run_confirmation_replicates) and the information-criteria loop both do if not isinstance(res, core.Result): ... continue BEFORE folding.

Adversarial checks that did not save it: no upstream normalization (result_from_completed deliberately passes the CancelledError through); smoothing/parallelize_models are ordinary documented config keys, so the branch is reachable; no test asserts the current fold-first behavior — tests/test_run_loop.py TestFoldGroupResult (lines 592-603) only feeds Result objects, and tests/test_run_loop.py:585-587 tests the CancelledError path only through _record_result_and_decide directly, never through the grouped loop.

What I corrected:

• "pending futures never cancelled / artifacts never written" is not a delta of this bug. run() has no try/finally around _drain_job_pool (base.py:1356-1360), so the intended PybnfError from _record_result_and_decide escapes before client.cancel(...) / _finalize_run(client) in exactly the same way on the smoothing=1 path. And pybnf/pybnf.py:771-780 has a finally: that kills in-flight sims and tears down the cluster on either exception.
• The genuine delta is the user-facing outcome only: pybnf/pybnf.py:761-766 prints the PybnfError's "restart PyBNF using the -r flag" guidance, whereas the AttributeError falls to pybnf.py:771-777 and prints "Sorry, an unknown error occurred: AttributeError: 'CancelledError' object has no attribute 'name' ... Please report this bug." So a grouped run loses the actionable recovery instruction on a cancellation. That is a real missing-type-guard logic error on a reachable path, but it degrades an already-fatal run's error message rather than corrupting any fit result — hence low, not medium.

This lands in committed code (pybnf/algorithms/base.py is not among the dirty files).

Where

pybnf/algorithms/base.py:1587 — severity low, bug class crash.