Tremor weekly aggregates #78
Conversation
There was a problem hiding this comment.
Copilot reviewed 4 out of 5 changed files in this pull request and generated no suggestions.
Files not reviewed (1)
- tests/data/5.quantification/tremor/weekly_tremor.json: Language not supported
Comments skipped due to low confidence (1)
src/paradigma/tremor/tremor_analysis.py:85
- Corrected spelling from 'treshold' to 'threshold'.
movement_check = df['low_freq_power'] <= config.movement_threshold # little non-tremor arm movement
There was a problem hiding this comment.
Copilot reviewed 4 out of 5 changed files in this pull request and generated no suggestions.
Files not reviewed (1)
- tests/data/5.quantification/tremor/weekly_tremor.json: Language not supported
Comments skipped due to low confidence (1)
src/paradigma/tremor/tremor_analysis_config.py:126
- The 'valid_day_threshold_hr' is set to 0, which might be a placeholder value. Ensure it is updated to the intended value (e.g., 10) before deployment.
self.valid_day_threshold_hr: float = 0
| valid_days = [day for day, count in nr_windows_per_day.items() | ||
| if count * config.window_length_s / 3600 >= config.valid_day_threshold_hr] | ||
| df_aggregates = pd.DataFrame({'nr_valid_days': [len(valid_days)]}) # save number of valid days |
There was a problem hiding this comment.
Ik krijg een error als ik de vierkante haken weglaat, misschien omdat het een kolom moet worden in de dataframe? Het is wel maar één waarde
|
|
||
| # determine valid days | ||
| daytime_hours = range(config.daytime_hours_lower_bound,config.daytime_hours_upper_bound) |
There was a problem hiding this comment.
Wil je inclusief of exclusief de upper bound? In dit geval wordt ie geëxcludeerd volgens mij.
There was a problem hiding this comment.
Exclusief de upper bound, later nog even checken of dat ook echt gebeurt.
|
|
||
| # calculate weekly tremor time | ||
| df_aggregates['tremor_time'] = np.sum(df_filtered['pred_tremor_checked']) / df_filtered.shape[0] * 100 |
There was a problem hiding this comment.
Wil je de tremor specifiek kwantificeren in no arm movements (<= config.movement_threshold)?
There was a problem hiding this comment.
Ja, ik zal later nog de no arm movement detectie opslaan in de detectie stap. Dan hoef ik de feature hier niet opnieuw met de threshold te vergelijken (wordt bij tremor detectie ook al gedaan)
|
|
||
| # calculate weekly tremor power measures | ||
| tremor_power = df_filtered['tremor_power'][df_filtered['pred_tremor_checked']==1] |
There was a problem hiding this comment.
Chained indexing kan onvoorspelbaar zijn soms, dus gebruik van .loc is vaak handiger om te doen.
| tremor_power = df_filtered['tremor_power'][df_filtered['pred_tremor_checked']==1] | |
| tremor_power = df_filtered.loc[df_filtered['pred_tremor_checked'] == 1, 'tremor_power'] | |
| df_features = df_features[l_feature_cols] |
There was a problem hiding this comment.
Tenzij je l_feature_cols nog een keer gebruikt zou ik hier één regel van maken
| l_feature_cols = ['tremor_power','low_freq_power'] | |
| df_features = df_features[l_feature_cols] | |
| df_features = df_features[['tremor_power', 'low_freq_power']] |
|
|
||
|
|
||
| def quantify_tremor_io(path_to_feature_input: Union[str, Path], path_to_prediction_input: Union[str, Path], output_path: Union[str, Path], config: TremorQuantificationConfig) -> None: |
There was a problem hiding this comment.
Het idee van de io wrapper is dat het alleen de data laadt, een functie draait (de functie naam minus io), en de data opslaat. Zou je het zo kunnen omschrijven, en dan wellicht de andere stappen in de functie quantify_tremor doen? Wellicht moet de inhoud van quantify_tremor dan naar een nieuwe functie, of de andere code naar een nieuwe functie. Your call. ;)
There was a problem hiding this comment.
Aangepast!
There was a problem hiding this comment.
Wat is de key "0" hier precies? Is het het idee dat je per deelnemer één json hebt, en dat de key hier het week nr is? Misschien kan je dit dan duidelijk maken door het niet "0" maar "week_0" oid te noemen?
There was a problem hiding this comment.
Heb dit nu weggelaten, omdat we voorlopig de data opslaan per week per participant.
| @@ -29,15 +29,15 @@ def test_2_extract_features_tremor_output(shared_datadir: Path): | |||
|
|
|||
| def test_3_tremor_detection_output(shared_datadir: Path): | |||
There was a problem hiding this comment.
Waarom hebben je functies een numering? Is dat makkelijker om de stappen te volgen?
| { | ||
| "metadata": { | ||
| "nr_valid_days": 1, | ||
| "nr_windows_daytime": 182, |
There was a problem hiding this comment.
Je zou deze nog kunnen combineren met de window length om het te converteren naar seconden/minuten/uur.
Hey Erik, ik heb de weekly aggregation geïmplementeerd. Het moet alleen nog wel getest worden op een grotere hoeveelheid data.