Support notation for unknown/ambiguous key or mode.

[PRamoneda]

key evaluation. Add: 'X' if the Key can't be categorized or 'other' if the mode can't be categorized.

This standar is followed by at least:

GiantSteps key EDM dataset Beatport key EDM dataset

The first one widely used in MIREX competition

[PRamoneda]

In future submissions, I will be more precise with the name of the branch.

[PRamoneda]

One moment I am editing the errors!

Thank you @lostanlen

Now the errors are fixed

[PRamoneda]

Hey @lostanlen, What do you think about a title like "Support notation for unknown/ambiguous key or mode."?

[lostanlen]

much better :)

[PRamoneda]

I have a question. I Giant steps there are keys that have two possible values (e. g. D Major | D minor), How would you make the evaluation?? @lostanlen

El 26 oct 2020, a las 16:39, Vincent Lostanlen notifications@github.com<mailto:notifications@github.com> escribió:

much better :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/craffel/mir_eval/pull/334#issuecomment-716631435, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AEAHMSGT4U37WYLXJ3JH3LTSMWJ3TANCNFSM4S24MJPQ.

[lostanlen]

@PRamoneda i recommend you ask the authors for this

[PRamoneda]

ok I will do that!

[craffel]

I don't know the convention so I'm not sure what the correct pattern is, but it seems cleaner if we require that "X" be accompanied by a mode (major/minor/other). For the datasets you mentioned, are uncategorizable keys annotated as "X" or "X other"? It seems like a strong assumption that any "X" key should be annotated as "X other".

[PRamoneda]

The keys are categorized as "X" in Giantsteps and Beatports datasets. I haven't find any example where modality were asigned to X.

BTW. Tomorrow, I will try to share a little colab for showing this PR across Giantsteps and Beatports Datasets.

[craffel]

I haven't find any example where modality were asigned to X.

Do you mean you haven't found any example where the modality was assigned to "other"? I thought X meant "uncategorizable key" and "other" meant "other modality". If there aren't any examples in any existing datasets/benchmarks where the modality is "other", I don't think we should support that modality label.

[PRamoneda]

I mean I haven't found any example with X major, X minor or X other. Always is "X" alone.

Beatport dataset and GiantSteps dataset have many tracks where the tonic is defined, and it is followed by "other" (Because the modality is ambiguous, for example,if the third degree is major and minor all the time).

For example, in giant steps: 31 keys are annotated with "tonic other" (The dataset have 600 songs).

[craffel]

Ok, if it's X alone, let's not use the convention where "other" gets added to X? Seems better to special-case "X" as not needing a modality.

[PRamoneda]

Giant steps have 19 "X" annotations.

[PRamoneda]

OK I will update the PR!

[PRamoneda]

ok! done @craffel and @lostanlen

[PRamoneda]

 # If reference or estimated key is x and they are not the same key
 # then the result is 'Other'.
    if reference_key == 'x' or estimated_key == 'x'\
            or reference_key == 'X' or estimated_key == 'X':
        return 0.

I can not use lower() because the key can be an integer(0-11) or a string ("X")

[craffel]

 # If reference or estimated key is x and they are not the same key
 # then the result is 'Other'.
    if reference_key == 'x' or estimated_key == 'x'\
            or reference_key == 'X' or estimated_key == 'X':
        return 0.

I can not use lower() because the key can be an integer(0-11) or a string ("X")

I don't think integer values for keys are allowed - where are you seeing that?

[PRamoneda]

Hey, @craffel The Key is declared as a string in the arguments. However, after ´key, mode´conversion it converts a key into a number.... (with the header dictionary, for computing intervals, and these things).

Thank you so much!

[craffel]

Hey, @craffel The Key is declared as a string in the arguments. However, after ´key, mode´conversion it converts a key into a number.... (with the header dictionary, for computing intervals, and these things).

Thank you so much!

But none of that happens before the if statement in question - the keys should still be strings at that point, right?

    validate(reference_key, estimated_key)
    reference_key, reference_mode = split_key_string(reference_key)
    estimated_key, estimated_mode = split_key_string(estimated_key)
    # If keys are the same, return 1.
    if reference_key == estimated_key and reference_mode == estimated_mode:
        return 1.
    # If reference or estimated key are x and they are not the same key
    # then the result is 'Other'.
    if reference_key == 'x' or estimated_key == 'x'\
            or reference_key == 'X' or estimated_key == 'X':
        return 0.

[PRamoneda]

ok you are right! I am sorry.

[PRamoneda]

@craffel I'm sorry to be such a pain. But I don't see it.

here the keys `reference_key, restimated_key' are converted from string to number. (if they are not 'X')

reference_key, reference_mode = split_key_string(reference_key)
estimated_key, estimated_mode = split_key_string(estimated_key)

and then in the if condition .lower() is not a number type function. And python launch an exception

[craffel]

@craffel I'm sorry to be such a pain. But I don't see it.

here the keys `reference_key, restimated_key' are converted from string to number. (if they are not 'X')

reference_key, reference_mode = split_key_string(reference_key)
estimated_key, estimated_mode = split_key_string(estimated_key)

and then in the if condition .lower() is not a number type function. And python launch an exception

I see what you are getting at - in that case I don't see how the check works at all, since split_key_to_string will return None for the key if it was originally X. So, checking if it is "X" or "x" doesn't make sense, you should check if it is None according to the convention you use in KEY_TO_SEMITONE

[craffel]

Thanks Pedro.

[PRamoneda]

Thanks to you! @craffel @lostanlen

El 30 oct 2020, a las 20:40, Colin Raffel notifications@github.com<mailto:notifications@github.com> escribió:

Thanks Pedro.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/craffel/mir_eval/pull/334#issuecomment-719758136, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AEAHMSDIWTOH626YO2BTY53SNMJCHANCNFSM4S24MJPQ.