binarybottle / engram

Arno's Engram v2.0 ("Engram") layout is an optimized key layout for touch typing in English based on ergonomic considerations, with a protocol and software for creating new, optimized key layouts in other languages.
MIT License
266 stars 24 forks source link

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

Open mateoinc opened 9 months ago

mateoinc commented 9 months ago

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.