search

onnela-lab / forest

Forest is a library for analyzing smartphone-based high-throughput digital phenotyping data
https://forest.beiwe.org
BSD 3-Clause "New" or "Revised" License
28 stars 16 forks source link

Jasmine updates #241

Closed clementzach closed 8 months ago

clementzach commented 9 months ago

Code to fix the jasmine out of range error

biblicabeebli commented 9 months ago

As far as I can go with visualizing how these offsets work: (longitude + 180) % 360 - 180 is completely fine because that is location on a circle, and should be handled as part of normal computation here.

For Latitude, we have the potential issue of "ticking up past 180" (uh, 360??) and suddenly the a correction clamping it by %360 swapping from north pole to south pole. (This issue is substantially unlikely to occur due to the restrictions on the magnitude of the offset - which I think is limited to 170 on both ios and android. (Literally I am bad at reading exactly that kind of behavior in code, it's just ... hard.) ---- I'm not seeing a corrective action on latitude.

I'm going to deploy this code to servers because I believe it resolves our current batch of errors.

I think this code needs to have tests for those cases where it could tick up and over. This is a safety thing, even if someone changes the code we need to know via a failing test that behavior has changed. We also need to somewhat more rigorously document the transformations occurring here in the code (and in the Android and iOS codebases) and declare why they are acceptable for Beiwe's purposes. For this reason I'm not going to merge it yet - we reference a commit over in the backend, not a branch name.

I'm slightly confused by one detail that came up in the email thread - it looks to me like our offset is no greater than one degree? that just seems wrong because it is ridiculously easy to de-anonamize - but literally this is not my field of expertise.

@clementzach do you want to set up a call so we can go through this verbally + with screen shares? It's very difficult to reason about without a secondary brain to sanity check, and particularly obnoxious to type out some of these details as text.

clementzach commented 9 months ago

My understanding is:

  1. Longitude (east-west) has valid values between -180 and 180. These can be changed any amount without any issue because moving North 1 degree at any point on the Equator is moving the same distance.

  2. Latitude (North/South) has valid values between -90 and 90. These cannot be changed very much because moving East 1 degree at the equator is a much bigger distance than moving East 1 degree at the North Pole. So, fuzzing for latitude is very small.

I think there may be more sophisticated methods of anonymizing data in the Beiwe app, but I think this method may be pretty close to good enough.

I agree with writing more unit tests and documentation. I'll send out an email to you and @jprince127 to set up a time.

clementzach commented 8 months ago

@jprince127 I added more documentation, I believe that this should be fine to merge unless you think there's something else that should be added