Tuplets / Triplets not fully supported

[andiejs]

\ QUICK FIX requested. Detailed instructions given. This is a half-hour job.

See graphics below for cases not handled: two notes of unequal duration that together form a triplet (or in the general case, n notes of unequal duration that together form an m-tuplet).

The tuplet module takes two integer prarmeters, num_notes and beats_occupied

Currently, parameter num_notes sets the number of notes to be affected, in this case 2, but also sets the glyph above the bracket, which should be 3 in this case.

Notice also that the scaling is incorrect so that notes sounding together are not printed vertically aligned. In this case, all note durations should be scaled to take up 2/3 of their usual horizontal space.

In the most general case, tuplets require three integers to be specified: the number of notes falling under the bracket, and the ratio that the notes are scaled by. In the most common case, the ratio is 2/x (i.e. quintuplets are scaled by 2/5). By convention, in the 2/x cases the bracket says only x (3 for triplets, 5 for quintuplets), otherwise the (inverted) scaling ratio is shown.

It would be possible to cover the great majority of useful cases with only two integers of input, by assuming that the numerator of the scaling ratio is 2. The module currently takes this numerator (as beats_occupied) but NOT the important denominator. It would be a very quick fix to the code to change the spec so that the second input "beats_occupied" functioned as the denominator here; both for glyph and scaling purposes.

The most general implementation would need to take both a numerator and denominator for the scaling ratio; the third graphic below shows a score by Stockhausen that requires this notation. This is correct, meaningful notation which should ideally be representable, but these are "advanced" cases that are not used nearly as often as those in the first graphics, which are extremely common and should definitely be represented.

It may be slightly more complicated to do this more general fix, since modules other than tuplet.js would need to be touched, as the input type to tuplet is changed, but the easier fix mentioned above would only affect this module, and current correct functionality would be maintained (e.g. num_notes = 3, beats_occupied = 2).

[0xfe]

Thanks for the report. That bottom image is just... wow?! :-)

I think adding that third parameter to the Tuplet is important, and also possibly straightforward. I'm not sure that I fully understand the space of possibilities with tuplets -- would you mind sending me a PR with this change? (I can try to figure out how other modules should incorporate this change.)

[gristow]

Looking at the present code, it would be possible to do this without adding the third parameter or any breaking changes:

• notes could accept notes of different durations, and
• we could calculate the ratio by summing the duration (in beats) of the notes passed and comparing it to options.beats_occupied.

Does that seem too magical? Maybe it's more obvious to accept the third parameter... though it would actually be redundant information.

[andiejs]

The bottom (Stockhausen) image has tuplets within tuplets, which are crazy, and I think it's more important to get non-nested, 3-parameter tuplets going. (I just realized that quintuplets are usually 5 in the time of 4, not of 2 as I said yesterday)

The possibility space (non-nested) is as follows: You already know about tuplets where they are all of equal length; e.g. five sixteenth notes in the time usually given to 4 sixteenth notes. Now these durations can be put together or divided in the usual ways: put two sixteenths together to get an eighth, three for a dotted eighth, or divide a sixteenth into two 32nds, etc. But no matter what durations we end up with under the tuplet, they will all be scaled to 4/5 of their notated value. Since the tuplet.js module doesn't check whether everything really adds up, all we have to do is give the scaling factor and specify which notes (i.e. how many notes) are scaled and bracketed.

Now the nested case: The top bracket in the Stockhausen image says 11:10, so all of the durations under it are scaled to 10/11 of their usual value. It's a measure of 5/4, so we should be able to sum up 11 eighth notes. However, the last 5 of those scaled eighth-notes are scaled again, by 5/7. So the outer tuplet is applied first, then the inner one. (*edited to add: since it's just multiplication, we can apply the inner tuplet first if we want to!)

I'd love to send a PR, but I have zero experience with javascript, and in trying to build your project on my system I am still at the stage of figuring out what node.js is. I was hoping that if I gave a good spec, it would be easy for someone else to implement it :-P

[andiejs]

Gristow: I'm not sure that "beats" are well defined here. Do we know whether we are talking about quarter notes vs eighth note beats?

[gristow]

You're right, the beats_occupied doesn't necessarily translate to beats at all in this context -- if you trace tuplets through to tickable, beats_occupied is used as the numerator and note_count as the denominator of the tick multiplier (the ratio).

So, for instance, in the case of beats_occupied = 2 and 3 notes of sixteenth note length, the tuplet will in fact occupy the space of an eighth note.

From a cursory glance at the code here's what I think needs to be done:

In tuplet.js and tickable.js:

• refactor beats_occupied to be ratioNumerator, override for old beats_occupied naming.
• refactor note_count to be ratioDenominator, override to calculate ratioDenominator if none is given and all notes are of the same duration.

In tuplet.js:

• allow notes of different durations
• implement "11:12" style bracketing when not a standard ratio (e.g. not 3:2, 2:3, 5:4, 7:8, etc...). I will take a look at Gould's Behind Bars in a few days (it's checked out of our library here at Oberlin at the moment), and see if she gives advice on which ratios to display.

If someone else wants to have a go at all this sooner, please do, otherwise I should have a pull request sometime this coming week-end.

[andiejs]

Thanks Gristow!

Right now note_count is serving a double function as the denominator of the scaling ratio AND the specifier for how many of the previous notes are affected. That's why I think we need a third parameter: ratioNumerator, ratioDenomenator, and note_count. If we get these three parameters going, I don't think it will matter whether the notes affected have different durations.

I believe right now "ratioed" is an input option deciding whether to print the (inverted) scaling ratio, or just the denominator. So it may not be necessary to decide automatically how to print.

[mscuthbert]

Mohit -- it gets worse than that bottom score... here's a solo cello piece: https://youtu.be/rW2b4ByT8dM?t=2m31s

[SalahAdDin]

Please update the tutorial with a more complex themes including this!

[0xfe]

@mscuthbert That totally looks like a circuit diagram. :-)

@gristow I'll be happy to take your PR.

[gristow]

Ok, great, @andiejs, do you see any problem with just doing:

var note_count = notes.length;

Since we have notes, I don't think we need note_count as a separate parameter, maybe not even as a variable. So, I'm thinking something like this:

/*
 * @param {Array.<Vex.Flow.StaveNote>}, the notes to be grouped together within this tuplet
 * @param options {
 *      tupletNumerator: play this many notes...
 *      tupletDenominator: ...in the space of this many.
 *    }
 */
function Tuplet(notes, options) {
    if (arguments.length > 0) this.init(notes, options);
}

[andiejs]

Oh, you're right @gristow I didn't realize we had the notes! That makes things way easier. As you say, we don't need note-count, only the two integers that make the ratio.

[0xfe]

Would it be useful to have the third parameter as an override? (e.g., for some weird corner cases)

On Mon, Mar 14, 2016 at 7:25 PM, andiejs notifications@github.com wrote:

Oh, you're right @gristow https://github.com/gristow I didn't realize we had the notes! That makes things way easier. As you say, we don't need note-count, only the two integers that make the ratio.

— Reply to this email directly or view it on GitHub https://github.com/0xfe/vexflow/issues/329#issuecomment-196563190.

Mohit Muthanna [mohit (at) muthanna (uhuh) com]

[andiejs]

I have been working on this some more, and now I think the current non-nested implementation is actually working properly (desipte the assertion in the code that it only works for tuplets with the same durations)-- can anyone confirm? (the js code i am using to interact with vexflow may be the culprit...)

I ran the attached code in the sandbox http://www.vexflow.com/docs/sandbox.html vextrip.txt

[gristow]

We are in far better shape than I realized. The existing code is quite good:

• Despite the confusing naming, num_notes actually refers to the tuplet ratio numerator.
• And, similary, beats_occupied refers to the tuplet denominator.
• Tuplet note groups with different rhythmic values function perfectly well, aligning correctly across multiple voices.
• Displaying "11:2" style tuplets is already possible by calling tuplet.setRatioed(true)
• Nested tuplets will result in a "voice does not have enough notes" or "too many ticks" error, because tickable.setTuplet first cancels any prior tuplets before applying the new tuplet. This is an easy mathematical fix, but requires more visual attention to avoid tuplet colisions:

Mainly we have a documentation problem. So, I'll submit a pull request with minor improvements:

• Add bracketed and ratioed as options in tuplet.js
• Add extensive commenting of code in tuplet.js, particularly concerning parameters that can be passed as options.
• Enable nested tuplets, and add a resetTuplet() call in tickable.js to return a tickable to its original intrinsic ticks.
• Add a y_offset option to tuplet to allow manual positioning of nested tuplets.

And I'll add an entry to the wiki describing all of the options that are available in tuplets.

[0xfe]

Wow, that's great to hear. Thanks for digging into this, Gregory! Also, good to hear that enabling nested tuplets is not that tricky (aside from the collision problem.)

[gristow]

Ok -- I've got a working pull request, and even added collision detection for nested tuplets, but cannot get images to generate to run visual regressions. (For some reason the XMLSerializer.serializeToString is returning '[ SVG ]' instead of the serialized SVG itself.)

Has anyone run into this issue?

Mohit, would you accept a PR without the regression tests?

[0xfe]

I haven't seen that error before. Yes, please send me a PR -- I'll run the tests here. Thanks!

(Also, would you mind patching in #324 and trying the regression tests.)

[gristow]

I patched in #324 and the regression tests work perfectly -- so glad to have the perceptual hashing. PR to come shortly.

[andiejs]

Thanks for this, guys! Looks amazing. Will be very useful for my project :)