Ejh 20170704 extended chord mapping #252
Merged
Conversation
This file contains 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
|
for convenience, I believe this line is that check. |
|
Yes, works for me! |
bmcfee
reviewed
Jul 12, 2017
| If a scale degree exceeds the length of the bit-vector, modulo the | ||
| scale degree back into the bit-vector; otherwise it is discarded. | ||
| length : int, default=12 | ||
| Length of the bit-vector to produce |
There was a problem hiding this comment.
also, does it make sense to support anything other than multiples of 12?
|
Thanks @ejhumphrey ! |
craffel
pushed a commit
that referenced
this pull request
Jun 22, 2018
* Updated test_chord to check for extended scale degrees. * Isolated extended chord reduction bug in scale_degree_to_bitmap, tests failing. * Fixes issue 251 * Removed crufty print statement
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.
Resolves issue #251.
Diagnosis: The method
scale_degree_to_bitmapinmir_eval.chordwas only ever dropping extended scale degrees on the floor, regardless of whether or notreduce_extended_chordswas set. This resulted in unexpected behavior whenreduce_extended_chords=True, because one expects that the extended scale degrees would be folded into the bit-vector representation.Fix: Two arguments were added to
scale_degree_to_bitmap:moduloandlength, defaulting toFalseand12respectively, which preserves the same behavior ofmir_evaloverall. Then, in theencodemethod,reduce_extended_chordsis passed on tomodulo. This also preserves the consistent behavior ofmir_eval, while fixing the expected behavior of these methods.This will be of interest to @fdlm and @bmcfee, having weighed in on the issue.