Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .agents/memory/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@
Project-shared agent memory for music21. One file per fact; see frontmatter for type.

- [Branches never to delete](branches-never-delete.md) — `master` and `m21_\d+` are permanent; never prune them
- [sortedcontainers for streams](sortedcontainers-for-streams.md) — tried twice to make streams always-sorted via SortedList; abandoned, too slow for tiny streams
- [Stream subclass generic defaults](stream-subclass-generic-defaults.md) — how `inherit-generics` should work once PEP 696 default type params land; maybe add typing_extensions
- [BFS-flatten-iterator](bfs-flatten-iterator.md) — lazy breadth-first flatten is writable (maybe for tree first), but must not reuse the flat/flatten name tied to offsetInHierarchy
36 changes: 36 additions & 0 deletions .agents/memory/bfs-flatten-iterator.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
name: bfs-flatten-iterator
description: Idea to flatten a Stream via a lazy breadth-first iterator instead of building a new Stream — viable, but must not reuse the flat/flatten name
metadata:
type: project
---

We once tried (the deleted `flatIterator` branch, Aug 2022) to turn
`Stream.flatten()` / the old `.flat` into a **lazy iterator** instead of
constructing a brand-new flattened Stream — a `FlatIterator` that yields elements
in order without materializing the container.

**Why it stalled:** the offsets. A flattened element's offset (relative to the
whole hierarchy) differs from its offset inside its own container. A materialized
flat Stream stores those recomputed `offsetInHierarchy` values; a lazy iterator
can't hold an offset independent of the site the element actually lives in. Making
that work was too hard, and the branch had to route around it
(`getElementsByOffset` → `getElementsByOffsetInHierarchy`). So *that* exact attempt
is shelved.

**But this is NOT a rejected idea** — call it the **BFS-flatten-iterator**. A lazy
breadth-first (or `iterateByOffset`) iterator that walks the hierarchy in order is
genuinely writable and could speed up flattening, *as long as it does not try to
set `offsetInHierarchy`*. The hard constraint:

- **Do not name it `flat` or `flatten()`.** Those names are too tightly bound to
the `offsetInHierarchy` contract; reusing them would re-create the blocker above.
Give the new thing its own name (`breadthFirstIterator` / `iterateByOffset`).

**How to apply:** when we need fast ordered traversal without the offset rewrite —
likely first in `tree/` — write a standalone `breadthFirstIterator` that skips
`offsetInHierarchy`, then potentially have `flatten()` build on it internally.
Treat the original `FlatIterator` as a fun leetcode-style exercise, not a design.
Temper expectations: a plain Python sort is already fast for under ~1000 elements
(the same small-N caveat that sank [[sortedcontainers-for-streams]]), so the
speedup may only matter on large flat scores.
59 changes: 59 additions & 0 deletions .agents/memory/sortedcontainers-for-streams.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
name: sortedcontainers-for-streams
description: Why we tried (and abandoned) backing Stream storage with the sortedcontainers library
metadata:
type: project
---

We twice prototyped replacing `StreamCore`'s hand-rolled sorted-insertion
bookkeeping with the `sortedcontainers.SortedList` library, on two now-deleted
branches:

- `sortedcontainers` — the full version. `StreamCore._elements` /
`_endElements` became `SortedList[ElementStore]`, where
`ElementStore = NamedTuple(sortTuple, element)` (added to `sorting.py`) pairs
each element with its `SortTuple` key so ordering is intrinsic to the
container. This let the entire `isSorted` flag machinery be deleted — no
`clearIsSorted`, no `ignoreSort` param, no `storeSorted` logic, no lazy
sort-on-access — shrinking `stream/base.py` by ~189 lines.
- `sorted_container_tree` — a narrower probe applying `SortedList` only to the
interval-tree internals (`tree/core2.py`) as a lower-risk place to validate the
approach first.

**The gain:** music21 streams would always be sorted by construction, removing a
whole class of "is this sorted yet?" state and the bugs that hide in it.

**Why we abandoned it:** `SortedList` is far too slow for *tiny* streams — the
overwhelmingly common case (e.g. a measure with 4 notes). The library's
B-tree-ish machinery only pays off at sizes music21 streams rarely reach, and the
per-insert constant-factor overhead is a net loss for small containers. The plain
`list` + append-at-end fast path beats it for real scores.

**How to apply:** don't reach for `sortedcontainers` (or a similar balanced-tree
container) to make streams always-sorted — it's already been tried twice and lost
on small-stream speed. If revisiting, benchmark the 2–8 element case first, not
just large streams. Both branches were deleted; the good ideas live here.

**Longer-term idea (Myke):** once `StreamCore` lives on `Stream` as `Stream.core`
(the deferred composition refactor), make two *swappable* `stream/core.py`
StreamCore implementations behind one shared abstract interface — the default
`list`-based `.core` for small streams, and a tree/B-tree or sortedcontainer
`.core` that a large stream can swap in. Each backing store is then best-suited to
its workload (tiny measures vs. huge flat scores) instead of forcing one container
to win both cases — which is exactly the trade-off that sank the all-or-nothing
attempts above.

**Related dead end — `bisect_getElements` (deleted):** a 2022 branch that kept the
plain `list` but replaced the linear scan in `getElementAtOrBefore` /
`getElementBeforeOffset` with a binary search (`bisect_right`/`bisect_left` keyed
on `elementOffset`). Because storage stays a list, it avoids the small-stream
*insertion* overhead that sank sortedcontainers — but it applied bisect
*unconditionally*, with no length gate, so tiny streams just ate the property-build
+ per-probe lambda cost for no gain. To be worth anything it would need to check
length first and only bisect the `classList=None` path (the `classList` path still
does an O(n) filter before bisecting, so it can't speed up anyway). And since a
binary search over a list won't actually beat the linear scan until well over
~1000 elements — a size real music21 streams almost never reach — **we should not
pursue bisect-on-list at all.** If we ever want fast offset lookup on big streams,
get it from the swappable-`.core` plan above (a sortedcontainer or binary-search
tree backend), not by bolting `bisect` onto the list.
33 changes: 33 additions & 0 deletions .agents/memory/stream-subclass-generic-defaults.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
name: stream-subclass-generic-defaults
description: How we want Stream subclasses to inherit the M21ObjType generic once Python typing allows default type params
metadata:
type: project
---

`Stream` is generic in its element type (`Stream[M21ObjType: base.Music21Object]`),
but its subclasses currently swallow the type parameter. The deleted
`inherit-generics` branch sketched the target shape: make every subclass carry the
generic through —

- `Voice`, `Measure`, `Part`, `System`, `Score`, `Opus`, `SpannerStorage`,
`VariantStorage` all become `Stream[M21ObjType]` instead of plain `Stream`, and
the `M21ObjType` / `StreamType` TypeVars in `common/types.py` become
`covariant=True`.

**The blocker (why it's deferred):** for this to be usable, the subclasses need a
*default* type argument — `Measure` should mean `Stream[Music21Object]` unless a
caller specializes it, not force every annotation to write `Measure[Note]`. Plain
generics give no way to default the parameter; that needs PEP 696 generic
defaults. The branch commit literally says "defer until default generic types are
allowed." This is the typing feature that lands natively in a future Python; it is
also available earlier via `typing_extensions.TypeVar(default=...)`.

**How to apply:** when default generic type parameters are usable in our supported
Python range, revive the `inherit-generics` shape (subclasses parameterized as
`Stream[M21ObjType]`, covariant TypeVars, per-subclass default of
`Music21Object`). Before then, investigate whether adding `typing_extensions` as a
dependency is worth it to get `TypeVar(default=...)` early — that would let us do
this without waiting for the Python floor to rise (and we must NOT raise the
`requires-python` floor to get it). Track alongside the other deferred typing
work.
2 changes: 1 addition & 1 deletion music21/analysis/windowed.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def process(self, subStream):
'''
Simply count the number of notes found
'''
return len(subStream.flatten().notesAndRests), None
return len(subStream.recurse().notesAndRests), None


class Test(unittest.TestCase):
Expand Down
Loading