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

Possible typo that changes the scoring function (though not the rankings) #75

Open
mateoinc opened this issue Feb 11, 2024 · 0 comments

Comments

@mateoinc
Copy link

Haven't thoroughly tested and I'm not 100% so writing an issue instead of a pull request, but on the engramv2 notebook, a few functions (including the ones that score layouts) have this normalization

newMax = 1
minF2 = np.min(F2)
maxF2 = np.max(F2)
newMin2 = minF2 / maxF2
F2 = newMin + (F2 - minF2) * (newMax - newMin2) / (maxF2 - minF2)

Notice that F2 is calculated with newMininstead of newMin2. It seems to me that this is wrong and should even fail (there's no newMin defined in the function), but goes unnoticed because of the normalization of the Flow and Strength matrices; as these are done in the global scope and with the variable newMin
I think this wouldn't change the rankings for the data used in this project because newMin2 will always be 0 (there's no 24 key layout that doesn't hit at least one 0 frequency bigram), so every F2 having the same newMin was going to happen anyways. But I haven't tested this. And this is definitely inflating the scores as the minimum values in the data_matrix are higher.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant