-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tutorial data preparation #103
Conversation
@@ -117,7 +124,8 @@ | |||
"metadata": {}, | |||
"source": [ | |||
"#### Set sensor values to the correct units\n", | |||
"First, TSDF stores the data efficiently using scaling factors. We should therefore convert the sensor values back to the true values. " | |||
"TSDF stores the data efficiently using scaling factors. We should therefore convert the sensor values back to the true values. This is only relevant if you use TSDF and scaled the data for storage purposes.\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only relevant if you use TSDF and scaled the data for storage purposes or also when you don't use tsdf but have scaled the data for storage purposes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's also relevant when using other data formats, but generally data formats such as parquet
have inherent data compression, meaning that the scaling happens when storing/loading. Therefore I don't expect users to require the scaling. Do you think we should remove this from the tutorial, and perhaps scale it prior to loading the data such that it doesn't distract the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not per se I think but I was just wondering if other formats have something like scale factors that you need to apply just like we do with TSDF. But if they all do this like Parquet then we can leave it this way.
@@ -171,7 +179,7 @@ | |||
"metadata": {}, | |||
"source": [ | |||
"#### Account for watch side\n", | |||
"For the Gait & Arm Swing pipeline, it is essential to ensure correct sensor axes orientation. For more information please read [X]. If the sensors are not correctly aligned, you can use `invert_watch_side` to ensure consistency between sensors worn on the left or right wrist." | |||
"For the Gait & Arm Swing pipeline, it is essential to ensure correct sensor axes orientation. For more information please read [Coordinate System](../guides/coordinate_system.md). If the sensors are not correctly aligned, you can use `invert_watch_side` to ensure consistency between sensors worn on the left or right wrist." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invert_watch_side does not necessarily correctly align the sensors right? It only does when one side is already correctly aligned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely correct, I'll adjust it accordingly.
@@ -198,7 +206,7 @@ | |||
"metadata": {}, | |||
"source": [ | |||
"#### Change time column\n", | |||
"ParaDigMa expects the data to be in seconds relative to the first row. The toolbox has the built-in function `transform_time_array` to help users transform their time column to the correct format." | |||
"ParaDigMa expects the data to be in seconds relative to the first row, which should be equal to 0. The toolbox has the built-in function `transform_time_array` to help users transform their time column to the correct format." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first row or first data point? The first is technically correct but I think the latter is more intuitive?
@Erikpostt I have some minor remarks regarding the data_preparation tutorial. The rest is looking good! I'll leave it up to you if you wish to make changes or merge the branch already ;) |
This PR changes the following: