cuthbertLab / music21

music21 is a Toolkit for Computational Musicology
https://www.music21.org/
Other
2.1k stars 397 forks source link

Notation routine for splitting notes in order to reinforce meter #992

Open Hwanggang opened 3 years ago

Hwanggang commented 3 years ago

music21 version

6.7.1

Problem summary

Converter midi to musicxml with one 4/4 bar ,which grouped by one sixteenth note and dotted eighth note and dotted test half note(tie pre eightth note) .

Steps to reproduce

Expected vs. actual behavior

Expected in Measure: expect

Actual in Measure : convert midi to xml like aa so musicxml show as actual

More information

jacobtylerwalls commented 3 years ago

Greetings, and thanks for taking the time to write. This is a known missing feature. Can be reproduced without external files like this:

>>> from music21 import *
>>> n = note.Note(quarterLength = 0.25)
>>> n2 = note.Note(quarterLength = 3.75)
>>> m = stream.Measure()
>>> m.append([n, n2])
>>> m.show()

Screen Shot 2021-04-30 at 8 28 01 AM

Mailing list discussion where @MarkGotham shared some code for handling this: https://groups.google.com/g/music21list/c/5G-UafJeW94/m/YcEBE0PFAQAJ

mscuthbert commented 3 years ago

I think that this is fine behavior. The input has two notes and the output is able to be represented as two notes, so it does so. Many composers (e.g., Bartok) prefer this output to the tied version, and there is an easy way to represent the tied version in musicxml. I'd lean towards not-fix.

jacobtylerwalls commented 3 years ago

Got it, but what about this case? Screen Shot 2021-05-03 at 8 44 07 AM

mscuthbert commented 3 years ago

I think that there's a need for a function to split that, but I don't think that it's makeTies() -- makeTies is for creating the minimum number of ties needed to make measures fit, etc. This might be a sort of meter.reduceNotatedSyncopation() or something like that. And there should presumably be different levels of syncopation to remove. For instance, does this reduce to8 8~2~8 8, or to 8 4.~4. 8, or to 8 8~4~4~8 8? And then imagine the same thing with 16 2.. 16 or something with tuplets. The number of "correct" simplifications gets enormous. Is the most important thing to show the highest level beat groupings or the lowest level or all of them? Do we weigh negatively number of ties/notes vs. difficulty of reading? And how do we make sure that 3/4 2~8 8 doesn't become 4~4~8 8 which seems much less readable to me.

It's a worthwhile exercise to consider implementing, and some level of it should go into MIDI parsing, but I don't want to change the makeTies behavior for this.

jacobtylerwalls commented 3 years ago

Yeah, there are always going to be multiple solutions. My inclination would be to define some violations to reexpress. For instance, when I taught music fundies and wanted to express this idea without introducing overwhelming detail to people just beginning to learn notation, I did something like: Make an effort to show beat 3 in 4/4 meter, but don't bother if:

That would cover the vast majority of MIDI files. Behind Bars has some more robust rules starting at p.166, but we wouldn't need to implement all of it.

MarkGotham commented 3 years ago

Thanks all, and for keeping sight of my proposed solution @jacobtylerwalls . Amazing that it's only been c. one year since this exchange ... seems much longer. I guess it's been a long year!

Totally agree with the emerging consensus that the best solution is to:

Looking again at that code I offered, the logic is currently:

So that's a strict implementation, but set up to make it easy to add a user control for relaxing the condition by ignoring certain metrical levels from consideration.

So in answer to @mscuthbert , solutions like:

Here's are some examples:

Ties

So then the user could be able to specify any option of that kind that's consistent with the time signature (raising an error otherwise), or to rely on the system's defaults, which could be set to something sensible like, say, 2 levels below that of the full metrical cycle so:

Worth emphasising that notes going from a strong to a weaker metrical position are unaffected (e.g. dotted note on the downbeat in simple time).

mscuthbert commented 3 years ago

great examples and I like the alternatives. The user's proposed answer (dotted 16th. + dotted half) should be something to consider also -- it has the advantage of showing the main beat of 4/4.

The Q of the interface for this is almost as hard as the implementation. What are these things called and how will people know how to use it? That's the part I'm banging my head on.

jacobtylerwalls commented 3 years ago

Maybe something similar to #845 where we override the base split() methods for Stream subclasses.

Maybe we need Stream.splitByQuarterLengths() where instead of splitting one huge note at a bunch of places, we split a stream's components at certain places. And then a new method to get the array of split points from the various config options.

jacobtylerwalls commented 3 years ago

Just discovered we have Stream.sliceAtOffsets(). I propose we get a noncontroversial version of it (splitting at 2.0 only in 4/4 meter etc) into the MIDI parsing flow and then we continue the discussion around creating a method to help people calculate the offsets they need to slice at for other cases, as Mark proposes above.

jacobtylerwalls commented 3 years ago

Worth emphasising that notes going from a strong to a weaker metrical position are unaffected (e.g. dotted note on the downbeat in simple time).

Mark, I'd be interested to hear how you accomplished this. Perhaps you can point to a snippet of code. If this is as elegant as it sounds, it's better than my special casing proposal for the 4/4 MIDI case.

In any case, yes, I would endorse getting your set of solutions into the system to generate a list of offsets and then pass them to sliceAtOffsets(). Would you be inclined to prepare a patch?

MarkGotham commented 3 years ago

Thanks @jacobtylerwalls !

I'm happy to finish off an implementation of this one (based on what I sent before) if that's ok by @Hwanggang and @mscuthbert.

In any case, I'll be in touch with a proposed implementation first, soon.

Hwanggang commented 3 years ago

Thanks @jacobtylerwalls & @mscuthbert . Thanks your suggestions. Thanks @MarkGotham . I am looking forward to your patch.

MarkGotham commented 3 years ago

Thanks @Hwanggang , all.

We have at least 3 challenges here as I see it:

  1. Accommodate the range of approaches discussed above (and more).
  2. Create a user-interface that's as easy to understand and use as possible. @mscuthbert is right to flag this as a significant part of the challenge.
  3. Avoid designing a system that's fundamentally limited to cases like 4/4, 6/8 and doesn't work for 5/X, 7/X etc. There are some temptingly quick and easy solutions that fall foul of this and could not practically be 'extended' later (i.e. would need to be completely re-written).

To solve for all of the above, I think we need a gathering stage defined by a list of in-measure offsets at which we're going to break up notes, and perhaps a few different ways for users to define this offset list. The vast majority of use cases will take those offset values from defaults defined in relation to standard metrical hierarchies, but in principle, advanced users with specific requirements should be able to jump in at any stage in the pipeline and specify anything they like.

To that effect, how about a single function with friendly defaults, but which also accommodates all realistic options by which users might want to set out their requirements and from which we can still easily deduce the equivalent list of offset positions. I.e. a function called something like splitNotesByMeter() within which there are different ways of defining the offset positions (all optional). Here are three possibilities, starting with the most direct (no conversion needed):

1. Offset Positions (I.e. directly)

2. Metrical Levels

3. Pulse Values

While 3 (metrical levels defined by easily recognisable note values) is probably more intuitive than 2 (in terms of an hierarchy), it's problematic for meters like 5/X et al. We could consider mapping un-dotted to dotted values and vice versa so that 1/4 would work directly in 4/4, via the dotted version in 6/8 and via both in 5/8, though that would require enforcing 2- and 3-groupings between metrical levels, and music21's MeterSequences are more flexible than that.

In summary and on balance, I'd be inclined to implement:

Speaking of that last point, @jacobtylerwalls this is handled by iterating over metrical levels. Only break if the note extends beyond a position on a higher metrical level (and that offset / level is included in the options).

Interested to hear what you think. If @mscuthbert gives the green light, I'll be happy to implement this when I can.

jacobtylerwalls commented 3 years ago

Super. Can you give me a quick demo of how we will derive the privileged status of semistrong beat 3 in 4/4? Right now I'm seeing no privilege:

>>> ts = meter.TimeSignature('4/4')
>>> ts.beatSequence
<music21.meter.core.MeterSequence {{1/8+1/8}+{1/8+1/8}+{1/8+1/8}+{1/8+1/8}}>
>>> ts.beamSequence
<music21.meter.core.MeterSequence {{1/8+1/8}+{1/8+1/8}+{1/8+1/8}+{1/8+1/8}}>
>>> ts.beatSequence.getLevel()
<music21.meter.core.MeterSequence {1/4+1/4+1/4+1/4}>
>>> ts.beatSequence.getLevel(level=1)
<music21.meter.core.MeterSequence {1/8+1/8+1/8+1/8+1/8+1/8+1/8+1/8}>
jacobtylerwalls commented 3 years ago

Ah! I suppose we can ignore the MeterSequences on the TimeSignature objects and just configure new ones?

>>> ms = meter.MeterSequence('4/4')
>>> ms
<MeterSequence {4/4}>
>>> ms.subdivideNestedHierarchy(1)
>>> ms
<MeterSequence {{1/2+1/2}}>
MarkGotham commented 3 years ago

Thanks @jacobtylerwalls . That's what I was thinking, though running that on 6/4 goes straight to the 1/4 note level {{1/4+1/4+1/4+1/4+1/4+1/4}} without the 2-part division. Similarly 7/8 heads straight for the 1/8th note. So it seems that method is missing anything between the measure and the 'beat' level (as defined by the 'denominator').

By contrast, meter.TimeSignature('6/8').beatSequence does give the division of 6/8 into 2, but (as you note above) it does not for 4/4.

Then there's cases like 5/4. Here, beamSequence will give the 2+3 division, though it insists on 2+3, even for meter.TimeSignature('3/4+2/4') due to the handling of 5 in _setDefaultBeamPartitions.

So unless I'm missing something, there isn't currently a list of divisions by level per meter, nor the functionality for making one. One way or another, I'd say we need that.

For the purpose of the issue at stake here, I'm inclined to go ahead with the logic above, assuming the existence of that offset list of lists by level for all supported meters while we work that out separately. In the meantime, for a range of simple cases, I could simply create and use a look-up directory of expected values (which can potentially also serve as a test case later).

E.g. given a timeSigStr like 4/4 and a levels list like [1, 2, 3] , to get the offsets for each level as a list of lists:

    ms = meter.MeterSequence(timeSigStr)
    ms.subdivideNestedHierarchy(max(levels))
    offsetByLevel = []
    for l in levels:
        offsetsThisLevel = [x[0] for x in ms.getLevelSpan(l)]
        offsetByLevel.append(offsetsThisLevel)

For 4/4, that gives you the right result: [[0.0], [0.0, 2.0], [0.0, 1.0, 2.0, 3.0], [0.0, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0, 3.5]] For 6/8 it currently returns: [[0.0], [0.0, 0.5, 1.0, 1.5, 2.0, 2.5], [0.0, 0.25, 0.5, 0.75, 1.0, 1.25, 1.5, 1.75, 2.0, 2.25, 2.5, 2.75], [0.0, 0.125, 0.25, 0.375, 0.5, 0.625, 0.75, 0.875, 1.0, 1.125, 1.25, 1.375, 1.5, 1.625, 1.75, 1.875, 2.0, 2.125, 2.25, 2.375, 2.5, 2.625, 2.75, 2.875]] The look-up tables would be broadly the same as these results, but slipping in the missing levels (in this case [0.0, 1.5] for the 6/8 case) and adjust the level counts accordingly.

jacobtylerwalls commented 3 years ago

The meter API is massive. I don't know it very well. I'm using this conversation to learn about it. (I hope asking my questions was useful!)

I see Mark pointing out taking a fresh MeterSequence and then jumping to a subdivision level doesn't work for ambiguously divided meters like 5/4 and 6/4, where the assumption is 'single':

>>> ms = meter.MeterSequence('5/4')
>>> ms.partitionStr
'Single'

But we do have:

>>> ms = meter.MeterSequence('5/4')
>>> ms.partitionByCount(2)
>>> ms
<music21.meter.core.MeterSequence {2/4+3/4}>
>>> ms.partitionStr
'Duple'

Maybe that could be used? If unpartitioned, partition into either 2 or 3, else use the default partition. Does that solve 6/8? If so, does that mean we don't have to build a key-value lookup?

Notice 3/8 isn't partitioned correctly: #925

jacobtylerwalls commented 3 years ago

Not trying to backseat drive, but just fleshing out my comment, this is perhaps more efficient:

>>> ms = meter.MeterSequence('6/4')
>>> ms.subdivideNestedHierarchy(0, firstPartitionForm=2)
>>> ms
<music21.meter.core.MeterSequence {{3/4+3/4}}>

But of course ...

>>> ms = meter.MeterSequence('9/8')
>>> ms.subdivideNestedHierarchy(0, firstPartitionForm=2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jwalls/music21/music21/meter/core.py", line 1000, in subdivideNestedHierarchy
    self.subdividePartitionsEqual(divFirst)
  File "/Users/jwalls/music21/music21/meter/core.py", line 908, in subdividePartitionsEqual
    self[i] = self[i].subdivide(divisionsLocal)
  File "/Users/jwalls/music21/music21/meter/core.py", line 253, in subdivide
    return self.subdivideByCount(value)
  File "/Users/jwalls/music21/music21/meter/core.py", line 170, in subdivideByCount
    ms.load(self, countRequest, autoWeight=True, targetWeight=self.weight)
  File "/Users/jwalls/music21/music21/meter/core.py", line 1171, in load
    self.partition(partitionRequest)
  File "/Users/jwalls/music21/music21/meter/core.py", line 863, in partition
    self.partitionByCount(value, loadDefault=loadDefault)
  File "/Users/jwalls/music21/music21/meter/core.py", line 686, in partitionByCount
    raise MeterException(
music21.exceptions21.MeterException: Cannot set partition by 2 (9/8)

We could just catch and then reattempt with 3.

MarkGotham commented 3 years ago

Thanks for this @jacobtylerwalls -- very helpful. I too am working out Chris' meter routines as I go. All suggestions very welcome -- feel free to take the wheel any time!

This might well provide enough functionality for us to get at the full metrical hierarchy. Whatever way we work this logic out, let's make it robust enough to offer to users directly with a single method call like .getFullMetricalHierarchy or the like, shall we? There are bound to be other users who will want to get straight to that info.

We'll also need to look carefully at cases like:

jacobtylerwalls commented 3 years ago

Great. I'd be happy to look in more detail once we have a patch to look at. I can help out with the MIDI parsing part.

Whatever way we work this logic out, let's make it robust enough to offer to users directly with a single method call like .getFullMetricalHierarchy or the like, shall we?

I'm open to this, just wondering if the idea is to avoid having the user specify firstPartitionForm=? Because I think that's all we're doing, checking if 1, trying 2, possibly reattempting with 3. Or is the motivation something like exposing it on TimeSignature so that users don't have to deal with the .beatSequence? I guess this depends on how much custom logic we end up writing (if we're successful in not altering a user's partition of 5/x then that's less custom logic!)

MarkGotham commented 3 years ago

Hi @jacobtylerwalls, all,

Please excuse the radio silence on this issue, I've only now managed to make some time for it.

https://github.com/MarkGotham/Misc/blob/main/breakItUp.py Here ^ is a draft solution that I think does a pretty solid job of

Basically,

I'm still working on this away from music21 to deal with the logical operations under discussion here separately from the question of how to integrate it and what changes the existing code base would need.

Those changes are are probably bugs that warrant a separate PR / issue. All the same, here are some notes on the current situation that may help to guide the best way forward.

Simple binary meters like 4/4:

Compound meters like 6/8 or 9/8

More 'complex' meters like 5/x and the like are not reliable.

My new (external) code handles all of these issues with a simple mapping of 'hidden layers'.

Finally, it's less clear even in principle how to handle cases like 4/4+3/8. We can discuss plausible defaults in music21, but this is perhaps one case that motivates the provision here for 'define your own metrical structure'.

Ok, that's the main issue. With that resolved, integration should be fine, and largely a question of direct swaps, e.g. with music21's offsetToSpan for iterating up the levels.

jacobtylerwalls commented 3 years ago

Hey Mark, thanks for this, and glad to hear of progress here.

I worked through the issues you presented, and I think they all have to do with the destructive-ness of partitionByCount(), which is also called by subdivideNestedHierarchy(). (Both methods mention destructiveness in the docs.) I think there should be a non-destructive way to subdivide, i.e., to retain the original partitions.

Regarding the 6/8 issue:

There is special casing in _setDefaultBeatPartitions() to arrive at the correct MeterSequences. In other words, we should just rely on the user's TimeSignature (which they could have carefully configured themselves in any case!):

>>> ms = meter.MeterSequence('6/8')
>>> ms
<music21.meter.core.MeterSequence {6/8}>
>>> ts = meter.TimeSignature('6/8')
>>> ts.beatSequence
<music21.meter.core.MeterSequence {{1/8+1/8+1/8}+{1/8+1/8+1/8}}>

I'll open an issue to track non-destructiveness in the partition routines, and then I would suggest you could work on the notation routine on the assumption that the partition routines get enhanced. Thanks again for moving this forward!

MarkGotham commented 3 years ago

Sounds great, thanks. So we can discuss non-destructive partition in the new issue, and keep this one for the note splitting routines specifically.

I welcome comments on that and / or challenging test cases for any (plausible) metrical routines that users would like to confirm are covered by the new code. I'm pretty sure it can deal with everything mentioned here so far.

My remaining question would be where to put the new code. Ultimately, I expect that most users will want to call it on a stream to edit all the metrical notation in a particular way.

jacobtylerwalls commented 3 years ago

Maybe Myke can weigh in, but I think it fits nicely with the makeNotation module, just like setStemDirectionForBeamGroups() in v.6.7. The other stream modules are getting too big, and this is about editing the notation.

jacobtylerwalls commented 3 years ago

@MarkGotham I will close the PR I just opened. I think this works just fine on master:

>>> fiveFour = meter.TimeSignature('3+2/4')  # get beatSequence from user's timeSignature
>>> bs = fiveFour.beatSequence
>>> bs
<music21.meter.core.MeterSequence {{1/4+1/4+1/4}+{1/4+1/4}}>
>>> for i in range(4):
...   c = copy.deepcopy(bs)  # since destructive, make a copy
...   c.subdivideNestedHierarchy(i, firstPartitionForm=bs)  # give it the partition we want to replicate
...   c.getLevel(i)
... 
<music21.meter.core.MeterSequence {5/4}>
<music21.meter.core.MeterSequence {3/4+2/4}>
<music21.meter.core.MeterSequence {1/4+1/4+1/4+1/4+1/4}>
<music21.meter.core.MeterSequence {1/8+1/8+1/8+1/8+1/8+1/8+1/8+1/8+1/8+1/8}>

The only thing it lacks is... the default beat partition for 4/4 (partitioned into half and half), which is what most users of this new functionality will need it for. Should it just be a keyword-arg? partitionSemiStrong? Then create the meterSequence ourselves instead of reading it from the time signature, and do:

>>> bs = meter.MeterSequence('2/4+2/4')
>>> bs
<music21.meter.core.MeterSequence {2/4+2/4}>
>>> for i in range(4):
...   c = copy.deepcopy(bs)  # since destructive, make a copy
...   c.subdivideNestedHierarchy(i, firstPartitionForm=bs)  # give it the partition we want to replicate
...   c.getLevel(i)
<music21.meter.core.MeterSequence {4/4}>
<music21.meter.core.MeterSequence {2/4+2/4}>
<music21.meter.core.MeterSequence {1/4+1/4+1/4+1/4}>
<music21.meter.core.MeterSequence {1/8+1/8+1/8+1/8+1/8+1/8+1/8+1/8}>

What do you think? 🤔