ELVIS-Project / vis-framework

Thoroughly modern symbolic musical data analysis suite.
http://elvisproject.ca/
31 stars 6 forks source link

Incomplete continuous n-gram annotations #318

Closed alexandermorgan closed 8 years ago

alexandermorgan commented 10 years ago

When using the "Continuous LilyPond" output format, n-grams that are followed by events that aren't n-grams (most often these are rests in one of the parts) lack an annotation for their second vertical interval. In the picture below the rest in the upper part on the downbeat of m. 3 causes the third 2-gram in m. 2 to lack an annotation for it's second vertical interval. The second measure's annotations should end with 6 -2 7 but the seven is missing. lassus7

crantila commented 10 years ago

Thanks---this is a very clean issue report.

For this "continuous LilyPond" situation, do you think we could include n-grams with rests? If we did, that would solve this problem quickly and easily... a "real" solution perhaps isn't worth the effort because we'll end up rewriting this algorithm for the VIS 2.x series.

alexandermorgan commented 10 years ago

Yes, I think that's a good quick fix for now. The problem will remain for the end of the piece, section divisions, etc. and ultimately displaying rests or not would be an optional setting but I totally agree, your solution is what we should go with right now.

crantila commented 10 years ago

Recap of a conversation Alex and I had.

Because this problem is caused by the NGramIndexer "terminator" setting when used to filter 'Rest' tokens from n-grams, this is where the solution is. For the "continuous lilypond" output method, we still want to use n-grams that have 'Rest' in them somewhere, but not the 'Rest' part itself. Thus, if the WorkflowManager's "include rests" setting is False, we're going to run the NGramIndexer without the "terminator" setting, then run another filter on its output to remove the 'Rest' tokens.

My plan is this: (1) I'll write that into the 'continuous_lilypond' branch, (2) I won't release it with VIS 1.2.8 because it's a new and untested feature, and (3) I'll merge 'continuous_lilypond' into 'devel_2', which will also get 'new_workflow_order' into 'devel_2'.

This allows us to have the fix now, as the professors requested, and to make the final VIS 1.x-series release before the end of August.

alexandermorgan commented 10 years ago

I thought our discussion was about a how best to fix the Continuous Lilypond output format. I still don't think that using the ngram indexer is a good idea because the desired output is not an ngram and that means that it's a bit of a fiddle to get it from the ngram indexer, if that will work at all. I think the best solution is to just zip together the results from the vertical and horizontal indexers in a manner similar to but different from that of the ngram indexer. Since LilyPond output takes a bit of time I think we should really avoid running extra indexers (in this case the ngram indexer) if they are not necessary. Perhaps we need a third opinion.

crantila commented 10 years ago

I guess we'll have to talk about this. I do still think the n-gram indexer is our best choice, and I think you may change your mind after I show you some recent experimentation I've done with that module. In other words, I think I've found a third option.

Regardless of this, a third opinion is indeed what we need!

crantila commented 9 years ago

Reassigning this to the "2.x LilyPond" milestone.

alexandermorgan commented 9 years ago

Didn't you already fix this @crantila ?

crantila commented 9 years ago

I think I only ended up fixing it in the "continuous_lilypond" branch.

alexandermorgan commented 8 years ago

These are now possible by using the new_ngram indexer with the 'open-ended' setting set to True.

musicus commented 8 years ago

Will this still be an issue once LilyPond is deprecated?

crantila commented 8 years ago

Looks like @alexandermorgan solved the problem in January anyway?

musicus commented 8 years ago

Let's close it!