diff --git a/.agents/memory/MEMORY.md b/.agents/memory/MEMORY.md index cac9ec283..2bcdd6844 100644 --- a/.agents/memory/MEMORY.md +++ b/.agents/memory/MEMORY.md @@ -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 diff --git a/.agents/memory/bfs-flatten-iterator.md b/.agents/memory/bfs-flatten-iterator.md new file mode 100644 index 000000000..60fde32e7 --- /dev/null +++ b/.agents/memory/bfs-flatten-iterator.md @@ -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. diff --git a/.agents/memory/sortedcontainers-for-streams.md b/.agents/memory/sortedcontainers-for-streams.md new file mode 100644 index 000000000..a866f1aed --- /dev/null +++ b/.agents/memory/sortedcontainers-for-streams.md @@ -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. diff --git a/.agents/memory/stream-subclass-generic-defaults.md b/.agents/memory/stream-subclass-generic-defaults.md new file mode 100644 index 000000000..d18e05d6b --- /dev/null +++ b/.agents/memory/stream-subclass-generic-defaults.md @@ -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. diff --git a/music21/analysis/windowed.py b/music21/analysis/windowed.py index f4c46b96e..64bead7e8 100644 --- a/music21/analysis/windowed.py +++ b/music21/analysis/windowed.py @@ -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):