-
Notifications
You must be signed in to change notification settings - Fork 8
Perception annotation #203
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
base: master
Are you sure you want to change the base?
Conversation
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.
Ну надо убрать эти изменения, к этому PR они никак не относятся
.gitignore
Outdated
| @@ -1,3 +1,6 @@ | |||
| .idea | |||
| venv | |||
|
|
|||
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.
Закинь сюда сразу .ruff_cache и .env (то что в perception у тебя). Тут еще ниже есть секция "Test data", туда можно добавить твои темповые папки
perception/.gitignore
Outdated
| venv | ||
| .env | ||
| __pycache__/ | ||
| .ruff_cache/ |
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.
Уже писал, кажется надо это дропнуть и слить с основным .gitignore
perception/README.md
Outdated
|
|
||
| ## 🛠️ Описание модулей | ||
|
|
||
| ### 3D_utils/mcap_to_pcd.py |
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.
Все что у нас под гитом находится мы описываем на english. Я не против если ты весь этот readme перенесешь в ноушен
perception/README.md
Outdated
| # Параметры в `config.py` | ||
| - `run_name` — название записи | ||
| - `frequency` — частота извлечения кадров из видео и конвертации в облако точек | ||
| - `elapsed_time` — сдвиг mcap записи в секундах (нужен для совмещения по времени pcd и mp4) |
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.
В моем понимании это должно лежать как .yaml файл, причем в идеале где-то рядом с исходниками (для разных проездов разные папки - и разные ямлики). А еще, как я понял, исходники у нас не под dvc сейчас. Может сложим?
perception/pcd_utils/pcd_to_ply.py
Outdated
| import open3d as o3d | ||
|
|
||
|
|
||
| def pcd_to_ply(pcd_file_path, ply_file_path): |
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.
Тут такой же вопрос
| from s3_storage.get_s3_conn import get_conn | ||
|
|
||
|
|
||
| def get_list_files_in_buckets(buckets: List[str], prefix: str | None = None): |
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.
Не понял зачем это, когда есть куча консольных утилит готовых (aws cli, s3cmd и т.п.)
perception/s3_storage/zip_on_s3.py
Outdated
| from s3_storage.get_s3_conn import get_conn | ||
|
|
||
| # Создаем сессию и клиент S3 | ||
| session = boto3.session.Session() |
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.
Я рекомендую на s3fs перейти, там api сильно приятнее
| :param metadate_file_path: путь к YAML-файлу метаданных | ||
| :param tz_name: название временной зоны | ||
| """ | ||
| with open(metadate_file_path, "r", encoding="utf-8") as file: |
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.
А зачем это из metadata.yaml читать? В mcap же по идее вся нужная информация есть
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.
И кстати раз такой код есть, я бы ожидал что для 360 видео ты тоже из метаданных достанешь таймстемп начала видео и таким образом обеспечишь автоматическую синхронизацию времени
perception/pipeline.py
Outdated
| shutil.make_archive(archive_path, "zip", root_dir=run_name) | ||
| # Удаляем исходную папку | ||
| shutil.rmtree(run_name) | ||
| print(f"Папка {run_name} архивирована в {output_dir}/{run_name}.zip и удалена") |
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.
Довольно странный скрипт если честно, я бы ожидал увидеть тут argparse и аргумент позволяющий выбрать id рана, который ты хочешь обработать
| client_kwargs={"endpoint_url": "https://storage.yandexcloud.net"}, | ||
| ) | ||
|
|
||
| bucket = "the-lab-bucket" |
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.
Хардкод
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.
Как уже говорил, вообще не вижу смысла в этом файле
| import yaml | ||
|
|
||
|
|
||
| def get_settings(run_name: str) -> Tuple[float, float, int]: |
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.
Очень странная функция...
- Я бы использовал более современный pathlib для взаимодействия с путями и файлами
- Вылидацию корректности yaml конфига логично делать через pydantic модельку
- Возвращаемое значение тоже логично сделать моделькой, а не тюпликами. Можно сделать эту модельку более насыщенной и использовать как глобальный "контекст" выбранного рана. То есть во все функции, где нужны определенные данные из конфига, передавать весь объект контекста, а не отдельные чиселки
| with open(input_mcap_file, "rb") as inp_file: | ||
| reader = make_reader(inp_file, decoder_factories=[DecoderFactory()]) | ||
| for schema, channel, message, ros_msg in reader.iter_decoded_messages( | ||
| topics=["/livox/lidar"] |
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.
Хардкод
| ) + timedelta(microseconds=timestamp_microseconds) | ||
|
|
||
| local_time = utc_time.astimezone(pytz.timezone(tz_name)) | ||
| return local_time.strftime("%Y-%m-%d %I:%M:%S.%f %p") |
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.
Только сейчас понял задумку этих функций, не надо так делать, все и всегда приводим к целому числу наносекунд в UTC зоне и дальше используем (в том числе для сравнений и т.п.)
Add annotation pipeline:
To fix: