Skip to content

Conversation

@dpascualhe
Copy link
Collaborator

Reverts #232 (reverts the revert 😄 🔁 )
Release for original DetectionMetrics: https://github.com/JdeRobot/DetectionMetrics/releases/tag/v1.0.0

A complete rewriting of DetectionMetrics focused on semantic segmentation (for now). The new approach is pure Python and provides classes that enable transparent access to different datasets (Rellis3D, GOOSE, our GAIA format) and tools for evaluation with different frameworks (PyTorch, Tensorflow).

More details are provided in the new README file.

Tech stack

  • Poetry for managing dependencies
  • Sphinx for generating automatic docs from docstrings

Pending infra

  • Create
  • Add tests
  • Github actions for:
    • Building and pushing detectionmetrics package to PyPI
    • Deploying Sphinx docs
    • Running tests

Pending functionality:

  • LiDAR segmentation
  • ONNX support
  • GUI for inspecting datasets and launching tests
  • CLI tool for launching tests

Things to decide:

  • What are we going to do with the previous docs? I thought about making a new entrypoint in https://jderobot.github.io/DetectionMetrics/ with info about both versions and the published article, and keep the old docs exactly as they are right now linked and accessible from a new URL (e.g. https://jderobot.github.io/DetectionMetrics/v1)

@dpascualhe
Copy link
Collaborator Author

I have modified the project webpage to have a new entrypoint, which clearly states the current status and acknowledges v1 appropriately. Also, the previous docs are untouched and will still be accessible in https://jderobot.github.io/DetectionMetrics/v1 (not deployed yet). Here is an overview of the new docs' appearance:

detection_metrics_docs.mp4

The new automatically generated sphinx docs for the Python code are stored in docs/py_docs and I've managed to make them accessible from the webpage when serving it with Jekyll locally (we'll see if it still works after deploying in Github pages).

I have also updated the README accordingly:
https://github.com/JdeRobot/DetectionMetrics/tree/revert-232-revert-231-dph/v2

@dpascualhe dpascualhe marked this pull request as ready for review November 22, 2024 15:34
Copy link
Member

@sergiopaniego sergiopaniego left a comment

Choose a reason for hiding this comment

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

Huge improvement!!
I've left some small comments 😄

README.md Outdated

### Project webpage [here](https://jderobot.github.io/DetectionMetrics)

*DetectionMetrics* is a family of toolkits designed to unify and streamline the evaluation of perception models across different frameworks and datasets. With the release of ***DetectionMetrics v1*** we introduced a versatile suite of tools focused on object detection, supporting cross-framework evaluation and analysis. [Cite our work](#cite) if you use it in your research!
Copy link
Member

Choose a reason for hiding this comment

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

Nice! We could move all the information related to v1 to the bottom of the README, with a clear reference or link at the beginning for easy navigation.

Since the paper references DetectionMetrics, we should make sure to include a mention of it in the README.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Can you check the new layout?


# DetectionMetrics

Copy link
Member

Choose a reason for hiding this comment

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

We could include an architecture diagram and a video of the application, as we had before. I think that's really useful for users.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added a simple diagram showcasing the classes included in the library. As soon as I have some GUI ready I'll post a video!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just upgraded it

Copy link
Member

Choose a reason for hiding this comment

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

This diagram looks awesome!!

README.md Outdated
<tr>
<th><i>DetectionMetrics v2</i></th>
<th>&#128187; <a href="https://github.com/JdeRobot/DetectionMetrics">Code</a></th>
<th>&#128295; <a href="/v2/installation">Installation</a></th>
Copy link
Member

Choose a reason for hiding this comment

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

I guess all this reference would work once we merge it

Copy link
Collaborator Author

@dpascualhe dpascualhe Nov 28, 2024

Choose a reason for hiding this comment

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

my bad, relative paths won't work in README 😅 I've fixed it but it won't work until new pages are deployed


### Common
Install your deep learning framework of preference in your environment. We have tested:
Copy link
Member

Choose a reason for hiding this comment

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

Is the application multiplatform?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in the sense of different OSs?

Copy link
Member

Choose a reason for hiding this comment

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

yep, it was an issue with the first version due to ROS constraints but not anymore as I can see!

from PIL import Image
import torch
from torch.utils.data import DataLoader, Dataset
from torchvision.transforms import v2
Copy link
Member

Choose a reason for hiding this comment

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

Reading the code below, I find this alias v2 a little strange.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, we now import v2 as transforms, which is more self-explanatory and resembles classic torchvision usage


# DetectionMetrics
Copy link
Member

Choose a reason for hiding this comment

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

Since we're generating the new version, maybe we could move the docs/gh-pages to a new branch gh-pages to separate the docs from the actual application code. What do you think?

Copy link
Collaborator Author

@dpascualhe dpascualhe Nov 28, 2024

Choose a reason for hiding this comment

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

I like the proposal. I'd try to merge it first and see if the current deployment works tho, because I have added plain HTML docs generated with Sphinx (docs/py_docs) and I'd like to check whether it's working or not (locally it does as shown in the video above)

Copy link
Member

@sergiopaniego sergiopaniego left a comment

Choose a reason for hiding this comment

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

LGTM! You can merge it 😄

Let's merge this version and start improving it instead of continuing working on this already big branch 🚀


### Common
Install your deep learning framework of preference in your environment. We have tested:
Copy link
Member

Choose a reason for hiding this comment

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

yep, it was an issue with the first version due to ROS constraints but not anymore as I can see!


# DetectionMetrics

Copy link
Member

Choose a reason for hiding this comment

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

This diagram looks awesome!!

@dpascualhe dpascualhe merged commit 8600121 into master Dec 2, 2024
@dpascualhe dpascualhe deleted the revert-232-revert-231-dph/v2 branch December 3, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants