Subcategory: error handling & robustness (8 → 10); also lifts overall architecture (9 → 10)
Problem
build_tree fails with a bare RecursionError on deep cluster trees, at universe sizes
that are ordinary for HRP. Single linkage is chaining-prone by construction, so on
decaying correlations it produces a tree of depth = n rather than O(log n).
Measured (correlations 0.99 ** |i-j|, method="single", bisection=False):
recursionlimit: 1000
n= 100 depth= 100 risk_parity -> ok
n= 500 depth= 500 risk_parity -> ok
n= 900 depth= 900 risk_parity -> ok
n= 1200 build_tree -> RecursionError
It fails during tree construction, before allocation is reached:
File "src/pyhrp/dendrogram.py", line 265, in build_tree
... <deep frames elided> ...
return Cluster(value=node.id)
File "src/pyhrp/cluster.py", line 139, in __init__
self.portfolio = Portfolio()
RecursionError: maximum recursion depth exceeded
1200 assets is a normal equity universe, so this is reachable in ordinary use rather
than only adversarially.
Why it matters beyond the crash
Every other failure path in this package raises a domain error with an actionable
message — dendrogram.py:187 even names the offending assets and explains that a
constant price series is the usual cause. RecursionError carries none of that: the
caller cannot tell whether the input was malformed, too large, or hit an internal bug.
The depth ceiling is also implicit in the tree contract rather than stated anywhere, and
four separate traversals inherit it.
Files / lines
src/pyhrp/dendrogram.py:213 — _to_cluster (recursive; the site that raises)
src/pyhrp/dendrogram.py:265 — the build_tree call into it
src/pyhrp/algos.py:185 — _allocate (recursive; same ceiling, reached later)
src/pyhrp/treelib.py:55 — Node.leaves (recursive)
src/pyhrp/treelib.py:106 — Node.size (recursive)
Note Node.levels (treelib.py:73) and Node.__iter__ (treelib.py:119) are already
iterative via deque — so the iterative pattern is established in the codebase.
Suggested direction
Either is acceptable; the second is strictly stronger:
- Guard. Detect the depth ceiling and raise a domain
ValueError naming the tree
depth, the asset count and the limit — matching the quality of the existing messages.
- Flatten. Convert
_to_cluster, _allocate, Node.leaves and Node.size to
iterative traversals, following the deque pattern already used by levels and
__iter__. This removes the ceiling rather than reporting it.
If the depth limit is instead considered acceptable, document it as a stated constraint
on build_tree so callers can size around it.
done when…
A chain-degenerate tree of at least 2000 assets under method="single" either allocates
successfully, or raises a domain-specific error naming the depth and the limit — and a
regression test covers that size. No code path in src/pyhrp/ reaches RecursionError
for a well-formed correlation matrix.
Evidence
n=1200 build_tree -> RecursionError, raised at src/pyhrp/dendrogram.py:213
(_to_cluster) via build_tree at :265. Default sys.getrecursionlimit() = 1000.
Subcategory: error handling & robustness (8 → 10); also lifts overall architecture (9 → 10)
Problem
build_treefails with a bareRecursionErroron deep cluster trees, at universe sizesthat are ordinary for HRP. Single linkage is chaining-prone by construction, so on
decaying correlations it produces a tree of depth = n rather than O(log n).
Measured (correlations
0.99 ** |i-j|,method="single",bisection=False):It fails during tree construction, before allocation is reached:
1200 assets is a normal equity universe, so this is reachable in ordinary use rather
than only adversarially.
Why it matters beyond the crash
Every other failure path in this package raises a domain error with an actionable
message —
dendrogram.py:187even names the offending assets and explains that aconstant price series is the usual cause.
RecursionErrorcarries none of that: thecaller cannot tell whether the input was malformed, too large, or hit an internal bug.
The depth ceiling is also implicit in the tree contract rather than stated anywhere, and
four separate traversals inherit it.
Files / lines
src/pyhrp/dendrogram.py:213—_to_cluster(recursive; the site that raises)src/pyhrp/dendrogram.py:265— thebuild_treecall into itsrc/pyhrp/algos.py:185—_allocate(recursive; same ceiling, reached later)src/pyhrp/treelib.py:55—Node.leaves(recursive)src/pyhrp/treelib.py:106—Node.size(recursive)Note
Node.levels(treelib.py:73) andNode.__iter__(treelib.py:119) are alreadyiterative via
deque— so the iterative pattern is established in the codebase.Suggested direction
Either is acceptable; the second is strictly stronger:
ValueErrornaming the treedepth, the asset count and the limit — matching the quality of the existing messages.
_to_cluster,_allocate,Node.leavesandNode.sizetoiterative traversals, following the
dequepattern already used bylevelsand__iter__. This removes the ceiling rather than reporting it.If the depth limit is instead considered acceptable, document it as a stated constraint
on
build_treeso callers can size around it.done when…
A chain-degenerate tree of at least 2000 assets under
method="single"either allocatessuccessfully, or raises a domain-specific error naming the depth and the limit — and a
regression test covers that size. No code path in
src/pyhrp/reachesRecursionErrorfor a well-formed correlation matrix.
Evidence
n=1200 build_tree -> RecursionError, raised atsrc/pyhrp/dendrogram.py:213(
_to_cluster) viabuild_treeat:265. Defaultsys.getrecursionlimit()= 1000.