Skip to content
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

Import data resampling #36

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

texhnolyze
Copy link
Contributor

@texhnolyze texhnolyze commented Nov 28, 2024

Summary

  • feat(db): setup sqlite WAL mode
  • fix(vscode): debug launch config
  • refactor(models): make joint states/commands fiels optional
  • feat(resampling): import data resampled

Proposed changes

This PR adds resampling with different converters by data type and different resampling strategies:

  • game_states are converted with original rate
  • images are converted with a max sample rate, meaning they can have a lesser rate, but not more, while not having consistent time deltas
  • joint_state and joint_command are resampled in sync with an interpolation using the last available value at any sample point

The initial db data point at t_0, having a relative timestamp of 0.0 is set only when all synced data becomes available (joint_state, joint_command, later also imu).
The last available game_state will also be saved at this time point t_0.

Before this, no data will be saved to the database.

The Converters currently still contain a mixture of bitbots/rosbag specific data access and conversion logic, which can be easily adjusted however by changing the fields of InputData to generic types to which we convert all of our different input data.
I think doing this abstraction makes most sense once we see the actual difference in data.

and other PRAGMA options for better paralle read/write performance
as we don't always have values for all joints and then want to default
to 0.0
@texhnolyze texhnolyze self-assigned this Nov 28, 2024
@texhnolyze texhnolyze force-pushed the feature/import-data-resampling branch 2 times, most recently from d585d15 to 5c3ce5b Compare November 28, 2024 17:21
with different converters by data type and different resampling
strategies:

- game_states are converted with original rate
- images are converted with a max sample rate, meaning they can have a
  lesser rate, but not more while not having consitent time deltas
- joint_state and joint_command are resampled in sync with an
  interpolation using the last available value at any sample point

The initial db data point at `t_0`, having a relative timestamp of `0.0`
is set only when all synced data becomes available (joint_state,
joint_command, later also imu).
The last available game_state will also be saved at this time point `t_0`.

Before this no data will be saved to the database.
@jaagut jaagut marked this pull request as ready for review November 28, 2024 17:24
converter = self.synced_data_converter
case "/DynamixelController/command":
last_messages_by_topic.joint_command = ros_msg
converter = self.synced_data_converter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no default case

from ddlitlab2024.dataset.imports.model_importer import InputData, Sample


class OriginalRateResampler:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create an interface for Resamplers

if num_samples > 0:
return samples
else:
self.last_received_data = data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you set last_reveived_data only here?

Comment on lines +37 to +39
self.last_sampled_data = self.last_received_data
self.last_sample_step_timestamp = self.last_sample_step_timestamp + self.sampling_step_in_seconds
samples.append(Sample(data=self.last_sampled_data, timestamp=self.last_sample_step_timestamp))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using class variables in Sample might be problematic if used by copy

else:
self.last_received_data = data
return [
Sample(data=self.last_sampled_data, timestamp=self.last_sample_step_timestamp, was_sampled_already=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please return empty list


@abstractmethod
def convert_to_model(self, data: InputData, relative_timestamp: float, recording: Recording) -> ModelData:
"""_summary_
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace summary

models = ModelData()

for sample in self.resampler.resample(data, relative_timestamp):
if not sample.was_sampled_already:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this?


if img_scaling_changed:
logger.error(
"The image sizes changed, during one recording! All images of a recording must have the same size."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"The image sizes changed, during one recording! All images of a recording must have the same size."
"The image sizes changed during one recording! All images of a recording must have the same size."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants