Add day segment analyses to GGIR part5 for WW and OO defined days - #1525
Add day segment analyses to GGIR part5 for WW and OO defined days#1525vincentvanhees wants to merge 11 commits into
Conversation
jhmigueles
left a comment
There was a problem hiding this comment.
Hi Vincent, thanks for this PR, I think this functionality was long expected. I have 2 main comments, probably related to my preliminary assumptions on what this would look like:
- I assumed that analysing segments in
"WW"or"OO"would allow for cross-midnight segments, as the analytical window usually includes midnight (it is not a set boundary as in"MM"). "WW"or"OO"windows could produce that a given segment appears twice in the same analytical window. You handled this by combining both segments into the same row in the reports (assigning the samesiindex), but I would have expected separated rows. My intuition is that a segment is a sub-window of consecutive epochs within the main window.
See specific comments on the code.
| } else { | ||
| qwindow = sort(qwindow) | ||
| if (qwindow[1] != 0) qwindow = c(0, qwindow) | ||
| if (qwindow[length(qwindow)] != 24) qwindow = c(qwindow, 24) |
There was a problem hiding this comment.
My preliminary asumption was that segmentation of "WW" or "OO" would allow cross-midnight segments. For example, a "WW" window from "2026-01-01 7:45:00" to "2026-01-02 8:00:00" would alow calculating a segment from "2026-01-01 23:00:00" to "2026-01-02 1:00:00". This line would not allow that as it would append a 24 after the last number in the case that qwindow = c(0, 23, 25). Window definition follows wake-up times but segments assume a strict 24-hour daily boundary, which feels counter-intuitive.
There was a problem hiding this comment.
Day segment definitions are still based on timeslot defined between midnight and midnight. None of the day segment analysis functionality in GGIR facilitates time segments that cross midnight. To facilitate that we would have to completely revise how daysegment analysis is done everywhere in GGIR, but that was not the goal for the project I did.
What we have had so far are MM day segments that do not care whether a segment is part of the present, previous or next sleep-window cycle.
What this PR adds is OO and WW day segments that make sure that only time points from the present sleep-wake or wake-sleep cycle are included. For example, when a person sleeps from 1:00 to 7:00 and the first window of the day is defined as 0:00-8:00. In the MM approach this would result in activity before and after sleep being combined into one segment. For some research questions this is problematic, they much rather have activity before 1:00 count towards the day that precedes sleep and activity between 7:00 and 8:00 count towards the day that succeeds sleep.
Which approach is best depends on the research question.
I will revise documentation to clarify this.
| ts$segment[segStart:segEnd] = si | ||
| for (gi in 1:Nsegments) { | ||
| if (!is.na(segStart[gi]) && !is.na(segEnd[gi])) { | ||
| ts$segment[segStart[gi]:segEnd[gi]] = si |
There was a problem hiding this comment.
If a segment occurs twice within the same biological window (e.g., in a "WW" window spanning from 7:00 AM on Day 1 to 9:00 AM on Day 2, a 7:00-8:00 AM qwindow interval appears twice), assigning the exact same segment index (si) to both non-consecutive blocks means GGIR treats them as a single segment.
Against my expectations:
- Instead of generating two separate rows in the report for each segment, it collapses them into a single summary row.
dur_day_spt_minwill aggregate both segments (e.g., showing 120 minutes instead of two separate 60-minute blocks). When I define a segment asqwindow = c(7, 8), I would not expect dur_day_spt_min go beyond 60 minutes.
Is it the intended design of this branch to merge recurring segments within extended windows, or should each segment be treated as a distinct entry in the subsequent analysis?
There was a problem hiding this comment.
This is intentional to achieve a constant number of segments across individuals.
If we would not do this then we end up with two, most likely partial, daysegments. It will make group level analysis more complicated when we start having a variable number of segments per person. Either way, it is not what the client wanted for this project, so I only focussed on the simple approach of having one segment per day per person.
With this PR it is now possible to perform daysegment analysis via parameter
qwindowwhen part5 is run with day definition WW and OO.Fixes #1407
Checklist before merging:
inst/NEWS.Rdwith a user-readable summary. Please, include references to relevant issues or PR discussions.DESCRIPTION,zenodo.json, andinst/CITATIONfiles.