cuthbertLab / music21

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

rntxt + anacruses + internal notes = conversion fail #1531

Closed MarkGotham closed 1 year ago

MarkGotham commented 1 year ago

music21 version

Latest, dev.

Problem summary

A few (c.1%) of apparently well-formed rntxt files fail to convert, and after a lot of counterfactual variant testing, the issue seems to be with the combination of anacrusis + internal notes.

By way of an example, here's a short song by Louise Reichardt https://github.com/MarkGotham/When-in-Rome/blob/master/Corpus/OpenScore-LiederCorpus/Reichardt%2C_Louise/Zwölf_Deutsche_und_Italiänische_Romantische_Gesänge/03_Durch_die_bunten_Rosenhecken/analysis.txt

Steps to reproduce

rntxt = """
Time Signature: 4/4
m0 b4 f: i
m1 ||: i b3 VI6
m2 i 
m3 iiø65 b3 viio7/V
m4 V
m5 V7 b3 V9
m6 V7 b3 viio43
m7 i6 b2 iv b3 Cad64 b4 V7
Note: repeats 3 times
m8 i :||
"""
s = converter.parse(rntxt, format="romanText")

This raises IndexError: tuple index out of range ... IndexError: attempting to access index -11 while elements is of size 10

Workaround

At present, I'm afraid I do not have a proper solution to report. If I work figure one out I'll PR it. Grateful for any others taking a look. And thanks to @malcolmsailor for confirmation/replication

mscuthbert commented 1 year ago

Thanks for the great bug report Mark! Can you simplify the file to the smallest one that causes the bug (removing any symbols that don't cause the crash?) -- For instance m2-m7 and replacing m1 with just a single token?

MarkGotham commented 1 year ago

For comparison, here's a couple of other failing cases (same issue):

Another song: https://github.com/MarkGotham/When-in-Rome/blob/master/Corpus/OpenScore-LiederCorpus/Mayer%2C_Emilie/3_Lieder%2C_Op.7/3_Wenn_der_Abendstern_die_Rosen/analysis.txt

A chorale: https://raw.githubusercontent.com/MarkGotham/When-in-Rome/master/Corpus/Early_Choral/Bach%2C_Johann_Sebastian/Chorales/098/analysis.txt

MarkGotham commented 1 year ago

Thanks for the great bug report Mark! Can you simplify the file to the smallest one that causes the bug (removing any symbols that don't cause the crash?) -- For instance m2-m7 and replacing m1 with just a single token?

Pleasure! And good shout!

rntxt = """
Time Signature: 4/4
m0 b4 f: i
m1 V
Note: Do not use internal notes with anacruses!
m2 i
"""
mscuthbert commented 1 year ago

thanks!

(also for the pieces, but the path to a fix is rarely through more complex cases but through simpler)

MarkGotham commented 1 year ago

Even smaller:

rntxt = """
Time Signature: 4/4
m0 b4 f: i
Note: Do not use internal notes with anacruses!
m1 V
"""
MarkGotham commented 1 year ago

thanks!

(also for the pieces, but the path to a fix is rarely through more complex cases but through simpler)

True, I guess my head is still in "identifying the issue" mode for which hundreds of complex example were needed! Unfortunately, I came across this in the midst of a 300 measure case in which there might have been any number of causes. Anyway, enough autobiography!

mscuthbert commented 1 year ago

ah! so nothing to do with the repeats!

Running right now in ipython

%debug romanText.translate.romanTextToStreamScore(rntxt)

which shortens the debug time a lot! It's in "fixPickupMeasure"

mscuthbert commented 1 year ago

Figured out the problem! whooy what a doozy:

we are iterating backwards from the end of the stream to move anything after the last measure backwards BUT then the stream gets sorted again so that we keep iterating over the same RomanTextUnprocessedMetadata object forever.

hard to diagnose. should be easy fix.

MarkGotham commented 1 year ago

Fantastic!

mscuthbert commented 1 year ago

fix is in as #1532 -- gotta go.