ucam-department-of-psychiatry / camcops

Cambridge Cognitive and Psychiatric Test Kit (CamCOPS)
Other
12 stars 8 forks source link

It is possible to enter zero height in the BMI task #366

Closed martinburchell closed 2 months ago

martinburchell commented 3 months ago

I've not investigated this fully but it looks like someone has managed to enter zero height in the BMI task and this has caused the server to throw an exception when dividing by zero. It looks like the client might crash as well.

martinburchell commented 3 months ago

The Linux client displays NaN for zero height. Might be the same on other platforms but we could guard against any possible crash by setting an unrealistic low minimum value.

RudolfCardinal commented 3 months ago

Agree entirely with modification to server. bmi.py.

In the client calculations, I think it's not guaranteed that the client couldn't crash, but almost guaranteed (perhaps compiler convention rather than absolute standard? e.g. https://stackoverflow.com/questions/4745311/c-division-by-0). So perhaps sensible to have a check in Bmi::bmiVariant() that returns QVariant() if height_m <= 0.0. If it's displaying NaN rather than "?", then I presume it's explicitly representing a NaN rather than a null QVariant(), so some of the other code might misbehave. But this code should cope with zero.

Then to avoid zeros getting in also -- yes, maybe; presumably would need a "minimum" parameter for QuHeight? For the Bmi task (and any others using this in practice) I imagine 0.1 m might be a sensible minimum.

I wonder why this happened, though; perhaps someone was insistent on using the BMI task to measure weight and didn't know height. In these circumstances, it might be more desirable to allow 0, producing a null BMI that is appropriately handled, than to enforce something duff (producing a numerically valid but nonsensical BMI). I suspect this is a realistic clinical scenario so perhaps we should roll with it. What do you think?

martinburchell commented 2 months ago

As per offline discussion, we'll still allow zero height but guard against it and look towards allowing patients to opt-out of tasks they do not wish to complete.