MathematicalMedicine / diver-issues

Semipublic tracking of issues for the DIVER front end
0 stars 0 forks source link

Minimums in ascertain_pedigrees seem to be swapped or overlapped #236

Closed WValenti closed 1 month ago

WValenti commented 6 months ago

In ascertain peds. If I ask for min 3 affecteds and min 4 individuals in ped, I see only peds with at least 4 affecteds. If I ask for min 3 affecteds and min 3 individuals, I do see peds with exactly 3 affecteds. So the "min individuals" constraint appears to be doing the same thing as the "min affecteds" constraint, at least in this case.

Viqsi commented 6 months ago

Fixed in MathematicalMedicine/diverRPC@6d15720

Viqsi commented 1 month ago

Well, crud. Referring to both of those parameters as "minimums" screwed up understanding and thus screwed up the fix.

There are THREE parameters involved:

We accidentally swapped not just which count is which, but also which count should be getting the user-specified constraint type. And when I "fixed" it, I swapped only the count parameters. D'oh!

Figuring this out obliged me to peek yet again at the original design doc in #134 to make sure it's supposed to be "min individuals" and "min/max/exact affected". Here it is as described by Veronica:

Include all pedigrees with [at least/at most/exactly] [2, 3,...] individuals with constraint = [show drop down list of values for selected constraining variable] and at least [2, 3,…] total number of individuals.

(There's also a specification in there for non-categorical/dichotomous variables, but that is unimplemented; we're Categorical/Dichotomous Only for now, still.)