Meter: Speedup with Immutable MeterTerminal and MeterSequence#1972
Draft
mscuthbert wants to merge 18 commits into
Draft
Meter: Speedup with Immutable MeterTerminal and MeterSequence#1972mscuthbert wants to merge 18 commits into
mscuthbert wants to merge 18 commits into
Conversation
…ration .duration on MeterTerminal/MeterSequence now returns a shared, immutable FrozenDuration, built once per (numerator, denominator) via an lru_cached static factory. This is groundwork for making the meter terminals/sequences themselves immutable and shared later; on its own it is performance-neutral (constructing and deep-copying meter objects are unchanged, since the Duration was already built lazily and terminals are still deep-copied). Public exposure is kept minimal: - TimeSignature.barDuration unfreezes back to an ordinary mutable Duration, so the common accessor is unchanged for callers (small copy cost). - The rarely-used beatDuration, beatDivisionDurations, getBeatDuration and Music21Object.beatDuration expose the FrozenDuration directly. Supporting changes so a FrozenDuration is transparently usable as a Duration: - Duration.__eq__ compares by musical value (isinstance + isGrace guard); a FrozenDuration equals the Duration it represents, a GraceDuration does not. - EqualSlottedObjectMixin.__eq__ returns NotImplemented on type mismatch, for symmetric value equality via reflected dispatch. - FrozenDuration.unfreeze() returns a mutable copy (sharing immutable slots, deep-copying only tuplets); augmentOrDiminish/splitDotGroups use it. - Music21Object may hold a FrozenDuration as its .duration (held shared, no client); the .quarterLength setter does copy-on-write. Bumps the beta version (b6 -> b7) to invalidate stale pickled-stream caches, since durations in cached parses can now be FrozenDuration. AI-assisted (Claude)
…ter deepcopy)
MeterTerminal is now an immutable value object defined solely by
(_numerator, _denominator, _weight): it subclasses common.objects.FrozenObject,
has no setters, and its duration is derived on demand from the cached
_durationFromNumeratorDenominator (the _duration/_overriddenDuration slots are
gone). Because it is immutable and shared:
- __deepcopy__ returns self, so deep-copying a MeterSequence/TimeSignature no
longer duplicates its terminals -- copy.deepcopy(TimeSignature('4/4')) is
~49% faster (0.205 -> 0.104 ms).
- getMeterTerminal(numerator, denominator, weight) returns one shared cached
terminal per value, so building meters reuses terminals -- construction is
~28% faster. "Changing" a terminal now means requesting the cached terminal
for the new values.
MeterSequence stays mutable: it keeps inheriting MeterTerminal but sets
frozen = False and reassigns __setattr__/__delattr__ to object's (avoiding
FrozenObject's per-set stack inspection on this hot, frequently rebuilt object),
and restores identity __eq__/__hash__ (its slots hold a list and a dict).
The ~10 places that re-weighted a terminal in place now replace it with a
cached, re-weighted terminal (MeterSequence.weight setter, load, getLevelList,
setLevelWeight -> new _applyLevelWeights; base.py _setDefaultAccentWeights and
setAccentWeight).
Bumps the beta version (b7 -> b8): MeterTerminal's slots changed, so older
pickled-stream caches are invalidated.
This is Phase A; a per-signature MeterSequence cache (so repeated TimeSignatures
of the same meter are a lookup) is the planned Phase B.
AI-assisted (Claude)
…s; doc fixes - MeterTerminal weight is now always a float (defaults are 1.0, annotations are float, and the terminal cache key no longer carries the weight's type). Marked as a subtle change on the weight property. No runtime coercion. - Immutable objects now also return self from __copy__ (not just __deepcopy__): MeterTerminal, FrozenDuration. MeterSequence overrides __copy__ to make a real independent copy (it is mutable). - Corrected the MeterTerminal.__init__ comment (all-default params are for MeterSequence's super().__init__(), not "how they are copied"), and trimmed the numerator/denominator/weight property docs to show how to make a new terminal. AI-assisted (Claude)
…4x faster)
Replaces the frozen=False code smell (a mutable MeterSequence subclassing the
frozen MeterTerminal) with a clean design: a shared MeterCore mixin holds the
read-only behavior (duration, ratioEqual, subdivide*), and MeterTerminal and
MeterSequence are now sibling immutable FrozenObjects -- MeterSequence no longer
inherits MeterTerminal.
MeterSequence is now a genuinely immutable value object: its _partition is a
tuple, it is hashable and compares by value, and __deepcopy__/__copy__ return
self. Its "mutating" operations (partition, partitionBy*, subdividePartitionsEqual,
subdivideNestedHierarchy, setLevelWeight, and the new replaceElement that supplants
item assignment) are now functional -- they return a new sequence and leave self
unchanged. Construction builds the immutable partition once (via the pure
_loadData helper) and freezes it.
getMeterSequence + a module cache return one shared object per value, so every
default 4/4 shares its beam/beat/accent/display sequences and the 2nd+
TimeSignature of a given meter is a cache lookup. TimeSignature (and the external
sites in analysis/metrical and braille/segment) became copy-on-write -- they
reassign a new immutable sequence instead of mutating in place.
Results: deep-copying a TimeSignature is ~4x faster than the previous commit
(~7.5x vs master); a MeterSequence deep-copies to itself; two TimeSignature('4/4')
now share their sequences. Full multiprocessTest (5285 tests), ruff, mypy, and
pylint all pass. Bumps b8 -> b9 (MeterSequence slots changed; caches invalidated).
AI-assisted (Claude)
- MeterTerminal.__init__: comment why object.__setattr__ is used (avoid FrozenObject's expensive per-set stack inspection), matching MeterSequence. - MeterSequence docstring: "hashable sequence of MeterTerminals" (not "list"). - Rename _withWeight -> public withWeight, documented as the immutable replacement for the pre-v11 `ms.weight = x` setter. - Add "New in v11" note to replaceElement (replaces `ms[i] = value`). - numerator/denominator/weight property docs: clearer wording and pass the weight through to getMeterTerminal in the examples. AI-assisted (Claude)
…licize makeSequence - Rename _makeSequence -> makeSequence (it is imported by meter/base.py, so it should not be private). - Add modify(**keywords) to MeterTerminal and MeterSequence: return a new object identical to self except for the named slot(s), like copy.replace (3.13+) / namedtuple._replace / sorting.SortTuple.modify. It is wired through the caches (getMeterTerminal / makeSequence), so identical results share one object. Also alias __replace__ = modify so copy.replace() works on Python 3.13+. - MeterTerminal.modify carries the other values over automatically, so the numerator/denominator/weight property docs now demonstrate it (e.g. a.modify(weight=0.25)) instead of a manual getMeterTerminal call. AI-assisted (Claude)
A Music21Object may hold a FrozenDuration as its .duration; it is up to whoever assigns it to unfreeze first if they intend to mutate it. So the quarterLength setter just sets self.duration.quarterLength -- which raises if the duration is frozen -- instead of silently replacing it with a mutable copy. Full test suite (5288 tests) passes unchanged. AI-assisted (Claude)
…doc polish - MeterSequence.replaceElement -> setIndex: it takes an index, not an element (and "replaceElement" collided conceptually with Variant.replaceElement, which does take elements). - Add meter tests for modify()/__replace__ and the terminal/sequence caches (identical values share one object; every default 4/4 shares its sequences). - test_base: a Music21Object may hold a FrozenDuration; setting .quarterLength on it now raises (was: copy-on-write) -- test asserts the TypeError. - base.beatDuration: doctest showing how to unfreeze a frozen beatDuration to modify it. - Document the removed methods in the MeterTerminal/MeterSequence class docstrings (setters; load(), weight setter, item assignment). - Trim the EqualSlottedObjectMixin.__eq__ NotImplemented comment. AI-assisted (Claude)
…onstruction) Deep-copy got much faster, but constructing a TimeSignature was actually slightly SLOWER than master: load() rebuilt the display/beam/beat/accent MeterSequences every time via the functional partition/subdivide ops, whose cache-key hashing (FrozenObject.__hash__ over the whole sequence tree) dominated -- the immutable dedup only shared the final objects, it did not skip the rebuild. Add _defaultSequenceCache keyed by (value, divisions): the fully-configured 4 sequences are immutable and already shared, and every change to a TimeSignature is copy-on-write, so a repeated meter can reuse them and skip the build entirely. Now the 2nd+ TimeSignature of a given meter is a lookup. Result: 1000 random TimeSignatures (4/4, 2/4, 3/4): ~52 ms (master) / ~63 ms (pre-cache) -> ~0.54 ms. Unhashable divisions (a list) fall back to no cache. Full multiprocessTest (5292) passes; ruff/mypy clean. AI-assisted (Claude)
… cache key) divisions may be list-like -- e.g. ['2/8', '3/8'] (Ariza & Cuthbert, ICMC 2010) -- so instead of skipping the cache for a list, normalize it to a tuple for the key. A list and the equivalent tuple now share one cache entry. Full suite (5292) passes. AI-assisted (Claude)
…opulated testMeterTerminalCache/testMeterSequenceCache previously only checked that two calls returned the same object; being internal tests, they now import the private _meterTerminalCache / _meterSequenceCache / _defaultSequenceCache and assert the returned objects are the ones stored under the expected keys. AI-assisted (Claude)
TimeSignature.barDuration currently unfreezes to an ordinary Duration for backward compatibility. Add a forward-looking doc note that it is expected to return an immutable FrozenDuration in v12, so callers should not rely on modifying it in place. AI-assisted (Claude)
The archetype cache in _setDefaultAccentWeights was the original construction speedup, but it is now superseded by _defaultSequenceCache (which caches the whole configured TimeSignature.load result per (value, divisions), so a repeat meter never runs _setDefaultAccentWeights at all). Its remaining benefit -- sharing an accent archetype across two different (value, divisions) that resolve to the same ratio/beat-count/depth -- is narrow and only on first construction. Removing it leaves a single caching mechanism. Full multiprocessTest (5292) passes; accent-weight computation is unchanged. AI-assisted (Claude)
…beat, accent) It always holds exactly four MeterSequences in that order, so annotate it as tuple[MeterSequence, MeterSequence, MeterSequence, MeterSequence] rather than a variable-length tuple. AI-assisted (Claude)
cddbbd3 to
6866822
Compare
The setIndex conversion of the old in-place b.beatSequence[0][i] = ... assignment had become a triple-nested one-liner. Replace it with a loop that builds up the new inner sequence in a variable and sets it back into beatSequence once, and document that it subdivides every component of beatSequence[0] by 2. AI-assisted (Claude)
modify now accepts numerator/denominator (rebuilding the sequence as that many
equal parts, each 1/denominator) in addition to partition/parenthesis/
summedNumerator; any other keyword (e.g. weight) is ignored. Still cache-wired.
e.g. MeterSequence('4/4', 4).modify(numerator=3) -> {1/4+1/4+1/4}.
AI-assisted (Claude)
…change num/den/weight Revert MeterSequence.modify to accept only partition/parenthesis/summedNumerator -- its numerator, denominator, and weight follow from the partition, so change them via partition()/subdividePartitionsEqual()/setIndex()/etc. Both MeterTerminal.modify and MeterSequence.modify now raise a ValueError on any keyword they do not handle, rather than silently ignoring it. Docs and tests updated. AI-assisted (Claude)
…rror message subdivideByOther only wrapped its argument in a single-element sequence (the partitionByOtherMeterSequence logic was long commented out), and nothing calls subdivide() with a MeterSequence, so both it and that dispatch branch were dead. Removed; noted among the removed methods in the MeterSequence class doc. Also trimmed the MeterSequence.modify() ValueError message to just the allowed keywords. AI-assisted (Claude)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TimeSignatures get Faster
Headline: TimeSignature objects change very little for standard users except their creation is sped up 100x after the first time signature of that type has happened. Time signature creation moves from 52µs to .54µs (540ns!).
Deepcopying TimeSignatures also got 10x faster, from 32µs to 3.3µs!
This moves one of the slowest parts of music21 to one of the fastest. It used to be that each TS took the time of 25 notes.
What changed probably doesn't affect you.
Only very powerful but less-often used features for changing the agogic accents, default beaming, and display of compound time signatures are significantly changed.
MeterTerminals and MeterSequences become immutable
TimeSignatures are made up of 4 MeterSequences (ways of organizing the meter, like into 4 beats in 4/4 = 1/4+1/4+1/4+1/4). Each of those is made up of other MeterSequences (for compound meter or complex organization) or eventually MeterTerminals (like 1/4).
These sequences are now immutable. So you can't divide (2/4+2/4) into (1/4+1/4+1/4+1/4) with partition(2) anymore. What you can do is from (2/4+2/4) use partition(2) to create a new MeterSequence as (1/4+1/4+1/4+1/4) and assign that where you want it. A small drawback for people working with MeterSequences that allows each time signature's default MeterSequences to be cached and reused.
to retrieve a MeterTerminal or Sequence from the cache use
meter.core.getMeterTerminalorgetMeterTerminalTimeSignature and Music21Object beatDuration becomes a FrozenDuration
Music21Object.beatDurationbecomes a FrozenDuration that cannot be manipulated. This allows it to share a duration with a MeterSequence and with all MeterSequences of the same meter (e.g. '4/4'). Callunfreeze()on it to get an unfrozen version().Same with
beatDivisionDurationsI purposely did not make
TimeSignature.barDurationa FrozenDuration in this PR since both inside and outside music21 it is known to be manipulated. This is expected to become a FrozenDuration in v12 or after.Consequences
ms = MeterSequence(); ms.load('4/4')fails. Just doms = MeterSequence('4/4')ms[0] = ms[0].subdivide(2)cannot modify in place:ms = a.setIndex(0, a[0].subdivide(2))(might change)mt = MeterTerminal('3/4'); mt.numerator = 2becomesmt = MeterTerminal('3/4'); mt = mt.modify(numerator=2)ts = meter.TimeSignature('6/8'); ts.beatSequence.partition(6);second statement becomests.beatSequence = ts.beatSequence.partition(6)Design and Future
Programmers who have worked with immutable Javascript web frameworks like React or Lit will understand this paradigm; it is however a little foreign to Python. There are no plans to make the whole music21 toolkit immutable by design -- the speedups are not worth the backwards incompatibilities. However, certain classes such as Microtones and Tuplets might migrate this way into the future (Tuplets attached to Durations are already immutable/frozen)
Misc
EqualSlottedObjectMixinobjects now return NotImplemented rather than False for comparison with unequal types._meterSequenceAccentArchetypesCache, which only was now going to come into play the second and subsequent time you created a 4/4 meter with a different accent pattern than default. It was the original cache taking construction from 2300 µs → 500 µs (each!), but I wanted to simplify where I could since I was adding complexity elsewhere.Started from an abandoned branch (meter-speed) from 2022; AI-assistance (Claude) helped me get the refactor over the edge.