opendr-eu / opendr

A modular, open and non-proprietary toolkit for core robotic functionalities by harnessing deep learning
Apache License 2.0
614 stars 95 forks source link

New Feature: Continual SLAM #424

Closed aselimc closed 11 months ago

aselimc commented 1 year ago

Hello everyone,

This PR aims to add new SLAM tool Continual SLAM, original repository can be found here.

aselimc commented 1 year ago

We have a quick question here to the repository owners. We are using g2o, for optimization; however, there is no setup.py for it. Thus, the installation process is a not straightforward. Here, should I try to write a setup.py file to do this installation automatically, or leave it out and just write how it should be done in README, or what would be your suggestion?

Thanks in advance

stefaniapedrazzi commented 1 year ago

We have a quick question here to the repository owners. We are using g2o, for optimization; however, there is no setup.py for it. Thus, the installation process is a not straightforward. Here, should I try to write a setup.py file to do this installation automatically, or leave it out and just write how it should be done in README, or what would be your suggestion?

Thanks in advance

Yes, you should write a setup.py file. We should aim at having a script that automatically installs all the tool's dependencies.

aselimc commented 1 year ago

@stefaniapedrazzi Hello, I have fixed the setup issue, and now it installs everything automatically. Can you please review the code?

Thanks

vniclas commented 1 year ago

Hi, following up on the email after the meeting. This PR has been ready for review since the last commit.

passalis commented 1 year ago

Thank you! Can you please update the dependencies to make sure that follow the rest of the toolkit. Currently we are using

torch==1.13.1
torchvision==0.14.1

You are specifying torch==1.9.0, but since we are having another version only one of the two is going to be installed, either breaking this tool or some of the rest of the tools.

Also, when not absolutely necessary, please consider using less strict versions, allowing for updates, i.e.,>= instead of == in dependencies.

aselimc commented 1 year ago

Hello @passalis , we have addressed everything. Please have a look at the draft again, thank you!

vniclas commented 11 months ago

@passalis @omichel Thank you for the approval!