Add fix for genea native dst support #211

angerhang · 2022-09-26T18:09:14Z

Currently GENEActive processing doesn't account for timezone or DST. This will create issues when the data was not recored in UTC.

This PR address the issue above by

Passing timeZone to the processing pipeline
Feed in the corrected timezoned UNIX time to the newValue function whose input is the true UNIX time

Test plan:

Verified with GGIR output is consistent with the current changes
DST test example

$ javac -cp accelerometer/java/JTransforms-3.1-with-dependencies.jar accelerometer/java/*.java
$ pip install --editable .
(env_name) hangy@NDPH8334 accelerometer % python accProcess.py ../1.bin 
Processing file '../1.bin' with these arguments:

activityClassification    : True
activityModel             : walmsley
calOffset                 : [0.0, 0.0, 0.0]
calSlope                  : [1.0, 1.0, 1.0]
calTemp                   : [0.0, 0.0, 0.0]
calibrationSphereCriteria : 0.3
csvSampleRate             : None
csvStartRow               : 1
csvStartTime              : None
csvTimeFormat             : yyyy-MM-dd HH:mm:ss.SSSxxxx '['VV']'
csvTimeXYZTempColsIndex   : 0,1,2,3,4
deleteIntermediateFiles   : True
endTime                   : None
epochFile                 : /Users/hangy/biobankAccelerometerAnalysis/1-epoch.csv.gz
epochPeriod               : 30
extractFeatures           : True
fourierFrequency          : False
fourierWithAcc            : False
inputFile                 : ../1.bin
intensityDistribution     : False
m10l5                     : False
meanTemp                  : None
mgCutPointMVPA            : 100
mgCutPointVPA             : 425
nonWearFile               : /Users/hangy/biobankAccelerometerAnalysis/1-nonWearBouts.csv.gz
npyFile                   : /Users/hangy/biobankAccelerometerAnalysis/1.npy
npyOutput                 : False
outputFolder              : /Users/hangy/biobankAccelerometerAnalysis
processInputFile          : True
psd                       : False
rawDataParser             : AccelerometerParser
rawFile                   : /Users/hangy/biobankAccelerometerAnalysis/1.csv.gz
rawOutput                 : False
resampleMethod            : linear
sampleRate                : 100
skipCalibration           : False
startTime                 : None
stationaryFile            : /Users/hangy/biobankAccelerometerAnalysis/1-stationaryPoints.csv.gz
stationaryStd             : 13
summaryFile               : /Users/hangy/biobankAccelerometerAnalysis/1-summary.json
timeSeriesDateColumn      : False
timeShift                 : 0
timeZone                  : Europe/London
tsFile                    : /Users/hangy/biobankAccelerometerAnalysis/1-timeSeries.csv.gz
useFilter                 : True
verbose                   : False

2022-09-27 11:44:06	=== Calibrating ===
Intermediate file: /Users/hangy/biobankAccelerometerAnalysis/1-stationaryPoints.csv.gz
Device was programmed with delayed start time
Session start: 2013-06-10T19:30:00.500+01:00[Europe/London]

2022-09-27 11:44:13	=== Extracting features ===
Intermediate file: /Users/hangy/biobankAccelerometerAnalysis/1-epoch.csv.gz
Device was programmed with delayed start time
Session start: 2013-06-10T19:30:00.500+01:00[Europe/London]

2022-09-27 11:44:34	=== Summarizing ===
Downloading https://wearables-files.ndph.ox.ac.uk/files/models/10Feb2022/walmsley/model.tar...
0 rows with NaN or Inf values, out of 1439

2022-09-27 11:45:03	=== Short summary ===
{
    "file-name": "../1.bin",
    "file-startTime": "2013-06-10 19:30:00.500000+0100 [Europe/London]",
    "file-endTime": "2013-06-11 07:29:00.500000+0100 [Europe/London]",
    "acc-overall-avg": 4.29104,
    "wearTime-overall(days)": 0.5,
    "nonWearTime-overall(days)": 0.0,
    "quality-goodWearTime": 0
}
Full summary written to: /Users/hangy/biobankAccelerometerAnalysis/1-summary.json

2022-09-27 11:45:03	In total, processing took 57.192051 seconds

Test file is from a sample Newcastle file /well/doherty/projects/newcastle_psg/acc/1.bin

Original acc plot

New acc plot with time zone and DST

The introduced change correctly move the time series forward by one hour in the summer. The the class prediction might be different possibly due to that the original plot was using an older version of bbaa. Based on the PSG data, this subject was awake around 07:30 which is consistent with the new processing. R stands for REM sleep and W stands for wake.

Disclaimer: Couldn't do a PR from my fork because my fork master has some important changes with the upstream doesn't have.

Resolves #203

Please could you review ASAP because I am blocked by the Geneactive data processing to retrain the model. Thank you :D

chanshing

Thanks @angerhang I just had one comment (see review)
Have you tried with a file with DST crossover in the other direction?

accelerometer/java/GENEActivReader.java

chanshing · 2022-09-27T15:52:33Z

@angerhang Actually before we merge this, I would like to test it with files where DST crossover actually happened (usually April and October). Can you do this? Or PM me with two such files for me to try.

angerhang · 2022-09-27T23:15:39Z

@angerhang Actually before we merge this, I would like to test it with files where DST crossover actually happened (usually April and October). Can you do this? Or PM me with two such files for me to try.

This is a good idea but unfortunately, we don't have access to any files that have DST crossover. https://zenodo.org/record/1160410#.YzODmC8w1qs Perhaps you are aware that there are some data files sitting around for Geneactive.

angerhang · 2022-09-27T23:17:52Z

Thanks @angerhang I just had one comment (see review)
Have you tried with a file with DST crossover in the other direction?

Yes I have done the crossover validation in the other direction with GGIR both in the winter and the summer.

chanshing · 2022-09-28T01:11:01Z

@angerhang Do you want to test your git-fu and rebase the latest changes in master onto your PR?

angerhang · 2022-09-28T09:58:58Z

I've rebased master into the branch. What's git-fu?

chanshing · 2022-09-28T11:23:19Z

I've rebased master into the branch. What's git-fu?

nvm! it's kung-fu but for git 😆

gmertes · 2022-09-28T11:31:51Z

I've rebased master into the branch.

FYI you've merged master into your branch, not rebased your branch onto master (hence the extra merge commit). No big deal, but a rebase would've been the 'clean' way. But Shing can now 'fix' it by doing a squash merge 🥳

chanshing · 2022-09-28T11:36:47Z

I've rebased master into the branch.

FYI you've merged master into your branch, not rebased your branch onto master (hence the extra merge commit). No big deal, but a rebase would've been the 'clean' way. But Shing can now 'fix' it by doing a squash merge 🥳

You read my mind :D I was just doing that + squashing both commits

angerhang · 2022-09-28T11:43:29Z

I've rebased master into the branch.

FYI you've merged master into your branch, not rebased your branch onto master (hence the extra merge commit). No big deal, but a rebase would've been the 'clean' way. But Shing can now 'fix' it by doing a squash merge 🥳

You read my mind :D I was just doing that + squashing both commits

oh I guess I am not to supposed to merge the PR? Or clicked the wrong button? lolol

angerhang · 2022-09-28T11:45:11Z

Oh you are referring to this? This is indeed better than having a merge commit. But it seems to be a new feature?

angerhang · 2022-09-28T11:46:10Z

Anyhow, thanks for reviewing the PR :D Now I am unblocked.

chanshing · 2022-09-28T11:52:55Z

@angerhang
No, it's from git. Instead of git merge master you could have done git rebase master. No worries though, I did it for you.

angerhang requested review from chanshing, aidendoherty and R-Walmsley September 26, 2022 18:09

angerhang self-assigned this Sep 26, 2022

angerhang marked this pull request as draft September 26, 2022 18:14

angerhang marked this pull request as ready for review September 27, 2022 10:55

angerhang added the priority label Sep 27, 2022

chanshing requested changes Sep 27, 2022

View reviewed changes

accelerometer/java/GENEActivReader.java Outdated Show resolved Hide resolved

angerhang requested a review from chanshing September 27, 2022 23:17

chanshing approved these changes Sep 28, 2022

View reviewed changes

Add fix for genea native dst support

b678413

chanshing force-pushed the tz-fix branch from 33d22d2 to b678413 Compare September 28, 2022 11:33

angerhang merged commit 7b7083c into master Sep 28, 2022

angerhang deleted the tz-fix branch September 28, 2022 11:40

angerhang restored the tz-fix branch September 28, 2022 11:42

angerhang deleted the tz-fix branch September 28, 2022 11:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fix for genea native dst support #211

Add fix for genea native dst support #211

angerhang commented Sep 26, 2022 •

edited by chanshing

Loading

chanshing left a comment •

edited

Loading

chanshing commented Sep 27, 2022

angerhang commented Sep 27, 2022

angerhang commented Sep 27, 2022

chanshing commented Sep 28, 2022

angerhang commented Sep 28, 2022

chanshing commented Sep 28, 2022

gmertes commented Sep 28, 2022

chanshing commented Sep 28, 2022

angerhang commented Sep 28, 2022

angerhang commented Sep 28, 2022

angerhang commented Sep 28, 2022

chanshing commented Sep 28, 2022

Add fix for genea native dst support #211

Add fix for genea native dst support #211

Conversation

angerhang commented Sep 26, 2022 • edited by chanshing Loading

chanshing left a comment • edited Loading

Choose a reason for hiding this comment

chanshing commented Sep 27, 2022

angerhang commented Sep 27, 2022

angerhang commented Sep 27, 2022

chanshing commented Sep 28, 2022

angerhang commented Sep 28, 2022

chanshing commented Sep 28, 2022

gmertes commented Sep 28, 2022

chanshing commented Sep 28, 2022

angerhang commented Sep 28, 2022

angerhang commented Sep 28, 2022

angerhang commented Sep 28, 2022

chanshing commented Sep 28, 2022

angerhang commented Sep 26, 2022 •

edited by chanshing

Loading

chanshing left a comment •

edited

Loading