Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] chord.evaluate with extension reductions. implements #274 #275

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bmcfee
Copy link
Collaborator

@bmcfee bmcfee commented May 4, 2018

This PR implements extension-reduction in the chord metrics.

It's currently WIP, as it has uncovered some undocumented quirks in chord eval that I'd like to get settled before we merge this.

@ejhumphrey
Copy link
Collaborator

ejhumphrey commented May 4, 2018

well then, i'm pleasantly surprised this works. lgtm so far

grumble chords grumble

@bmcfee
Copy link
Collaborator Author

bmcfee commented May 4, 2018

well then, i'm pleasantly surprised this works. lgtm so far

grumble chords grumble

The strange thing is that the test case hits on thirds, triads, and tetrads, but fails only on sevenths. (And majmin, I guess, maybe we should consider changing that one...)

@bmcfee
Copy link
Collaborator Author

bmcfee commented May 4, 2018

(warning: nitpicking to follow. sorry!)

I'm going through the chord metrics implementation, just to see if there are any other likely oddities. Here are some things I've noticed that seem strange:

  • thirds should this not be matching on [:, 3:4] instead of [:, 3]? Should it be including the 2nd (or 9th) in this check? I always understood this metric as est_root == ref_root and est_third == ref_third (regardless of other tones).

  • triads similar comment: why are we looking at the 2nd and 4th here (implied by [:8])? Shouldn't this just depend on (root, m3, M3, TT, P5, m6) bins?

  • majmin seems like the idea here is to only look at things below the 6th, and check for exact matching against the maj/min qualities. As noted in chord.eval: expose extension-reduction parameters #274, this gets dicey with extension reduction. Maybe that's fine?

  • sevenths missing some qualities. As noted in chord.eval: expose extension-reduction parameters #274, this one behaves differently from triads and tetrads, which is concerning. Triads counts matches if the ref and est agree to the first 8 positions, tetrads does the same but for all positions. Sevenths breaks this pattern, and works more like the maj/min metric (check for equivalence to one of the template profiles).

@bmcfee bmcfee mentioned this pull request Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants