wadpac / GGIR

Code corresponding to R package GGIR
https://wadpac.github.io/GGIR/
Apache License 2.0
94 stars 60 forks source link

Improve handling of NotWorn with second guider (HorAngle or HDCZA) #1186

Closed vincentvanhees closed 2 weeks ago

vincentvanhees commented 1 month ago

Earlier this year in PR #1137 I added the option to specify HASPT.algo = c("NotWorn", "HDCZA") or HASPT.algo = c("NotWorn", "HorAngle"). If the window has more than 25% nonwear it will use NotWorn algorithm, alternatively it will fall back on the second guider.

In this PR I am making a number of additional updates:

  1. The guider as used, which can differ from night to night, now correctly logged in csv reports (see changes to g.part4).
  2. Nights for which NotWorn is used are now skipped in the csv part4_nightsummary_sleep_full.csv report (see changes to g.report.part4).
  3. HorAngle threshold increased to 60 as that seems more suitable. Additional updates may follow later this autumn but for now this seems a safer choice than 45. As a result I also had to update test_recordingEndSleepHour.
  4. HASPT.ignore.invalid is now automatically set to NA when NotWorn guider is used. In that way user has freedom control it in relation to HDCZA or HorAngle, while sticking to NA for NotWorn.
  5. Fix to g.part5.savetimeseries that failed to log the guider used when it includes +invalid in the name.
  6. The selectdaysfile functionality, that got deprecated several years ago, was still present in the unit test, now removed.

Closes #1156

Checklist before merging:

vincentvanhees commented 1 month ago

Hi @jhmigueles When you are back from your summer break it would be good if you could have a look whether you are happy with this PR. It relates to the German project.

jhmigueles commented 3 weeks ago

I am working on this today, running now on a large dataset, will inspect the output later today and get back before the end of the day.

jhmigueles commented 3 weeks ago

Thanks Vincent! I think everything works as expected, I can see the guider correctly specified for every night in part 4 reports and in the part 5 time series. In your comment above, it says

Nights for which NotWorn is used are now skipped in the csv part4_nightsummary_sleep_full.csv report (see changes to g.report.part4).

You mean in the part4_nightsummary_sleep_cleaned.csv, right? In my tests, the nightsummary_full contains all nights, including those with the NotWorn algorithm, but I think this is the expected behavior. If so, I do not have any comments, and I think this is ready to be merged.

vincentvanhees commented 2 weeks ago

You mean in the part4_nightsummary_sleep_cleaned.csv, right?

yes that is correct