Arm swing quantification #94
Conversation
…/paradigma into minor-pipeline-updates
…minor-pipeline-updates
…minor-pipeline-updates
…arm-swing-quantification
|
@nienketimmermans ready for review! |
There was a problem hiding this comment.
@Erikpostt Looking good! Only some things to check, but after that it's ready to be merged.
| "\n", | ||
| "preprocess_imu_data_io(\n", | ||
| " input_path=path_to_sensor_data, \n", | ||
| " output_path=path_to_preprocessed_data, \n", |
There was a problem hiding this comment.
For consistency you can change this to path_to_input and path_to_output
| @@ -236,7 +234,7 @@ def compute_mfccs( | |||
| - sampling_frequency : int | |||
| Sampling frequency of the data (default: 100 Hz). | |||
| - mfcc_low_frequency : float | |||
| Lower bound of the frequency band (default: 0 Hz). | |||
| Lower bound of the frequency band (default: 0). | |||
There was a problem hiding this comment.
Why did you remove Hz here?
There was a problem hiding this comment.
I honestly have no idea how that happened, perhaps because of other intermediary changes (it was config.X for a while).
src/paradigma/gait/gait_analysis.py
Outdated
|
|
||
| The input DataFrame with two additional columns: | ||
| - `gait_probability`: The predicted probabilities of gait activity. | ||
| - `gait_detected`: A binary classification result (1 for gait detected, 0 otherwise) based on the threshold. |
There was a problem hiding this comment.
Only gait_probability is returned right? And it's not added to the input dataframe or am I wrong?
There was a problem hiding this comment.
Correct, I forgot to update the docstrings and parameter types it seems.
src/paradigma/gait/gait_analysis.py
Outdated
| Returns | ||
| ------- | ||
| dict | ||
| A dictionary containing the quantified arm swing parameters for each segment of motion. |
There was a problem hiding this comment.
You mean arm swing motion?
There was a problem hiding this comment.
Sharp, indeed the docstring was incorrect here.
There was a problem hiding this comment.
Are the variance and scale correct?
| "end_iso8601": "2021-06-27T17:04:02Z", | ||
| "rows": 130, | ||
| "end_iso8601": "2021-06-27T17:03:59Z", | ||
| "rows": 69, |
There was a problem hiding this comment.
There are less rows now, is that correct?
|
@nienketimmermans this PR is ready for review again, thank you for your comments and suggestions. I have updated the scaler, which was indeed incorrect. I have also made a distinguishment between quantification and aggregation by adding the aggregation step to the pipeline. Finally, the reduction in number of rows of gait without other arm activity is not an issue, this is primarily due to changes in the features and classifiers, and the test data is not representative of a true sample. Due to the lack of predicted arm swing in the test data segment, there are also no quantification values in the resulting json. |
There was a problem hiding this comment.
@Erikpostt Looking good! It's ready to be merged from my side.
This is the final piece of the gait pipeline, containing not just the arm swing quantification but also several additional features/improvements.
Contains
Arm swing quantification
The following was added:
Note 1: The quantification changed from window-based to timestamp-based. This simplified the process of eliminating duplicate peaks due to overlapping windows.
Note 2: The quantification step stores a json file with all range of motion and peak velocity values. Later, we may opt for including aggregations for users (e.g., median or percentiles).
Flexibility for users
For several functions I opted for specifically requesting input of parameters instead of simply requesting a config object. This allows users to more easily call these functionalities, as I consider these functionalities to not just be a component of the pipeline, but also a seperate functionality that can be used outside of the pipeline.
Adjustment of parameters and docstrings
I have added docstrings where they were missing or expanded them. I also modified parameter types using "|" instead of "Union" according to PEP8 standards.