-
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
368 visionnode and segmentation #404
Conversation
WalkthroughThe changes introduce a new markdown file, Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
doc/research/paf24/segmentation.md (3)
11-17
: Enhance documentation with proper context and formatting.The section needs improvement in the following areas:
- Add context explaining what these solutions are and how they work
- Format links with descriptive text instead of bare URLs
- Explain the relationship between the implementation and the dataset generator
Example improvement:
## Already implemented solutions The project currently uses a segmentation model that processes camera input for detecting road features. The model implementation can be found in the [perception launch configuration](https://github.com/una-auxme/paf/blob/c3011ee70039e199e106c54aa162a8f52be241a6/code/perception/launch/perception.launch?plain=1#L59-L61). The model was trained using a custom dataset generated through our [dataset generator](https://github.com/una-auxme/paf/blob/8e8f9a1a03ae09d5ac763c1a11b398fc1ce144b0/code/perception/src/dataset_generator.py#L109-L110), which processes raw sensor data to create training samples.🧰 Tools
🪛 Markdownlint
13-13: null
Bare URL used(MD034, no-bare-urls)
16-16: null
Bare URL used(MD034, no-bare-urls)
18-20
: Document the rationale for dropping EfficientPS.Please provide context about:
- Why EfficientPS was initially chosen
- What issues or limitations were encountered
- The decision-making process that led to dropping it
This information is crucial for future reference and helps prevent re-investigating already explored solutions.
🧰 Tools
🪛 Markdownlint
18-18: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
18-18: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
19-19: null
Bare URL used(MD034, no-bare-urls)
40-41
: Address architectural decision and fix formatting.The question raises important architectural considerations that should be addressed explicitly.
Please:
- Document the trade-offs between bounding boxes and segmentation models:
- Performance characteristics
- Accuracy comparisons
- Resource requirements
- Include benchmarks if available
- Document the reasoning behind the final decision
Also, fix the formatting issues:
-## Follow up Question: -Why the last group used bounding boxes and not the segmentation model is it to slow or maybe not reliable? +## Follow-up Question + +Why did the previous group choose bounding boxes over the segmentation model? Is it too slow or perhaps not reliable enough?🧰 Tools
🪛 LanguageTool
[grammar] ~40-~40: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...n walks will be labeld as roadlines ## Follow up Question: Why the last group used bound...(CA_FOLLOW_UP)
[uncategorized] ~41-~41: Possible missing comma found.
Context: ...bounding boxes and not the segmentation model is it to slow or maybe not reliable?(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~41-~41: “to” seems less likely than “too” (as well, very, ‘too much’).
Context: ...es and not the segmentation model is it to slow or maybe not reliable?(AI_HYDRA_LEO_CP_TO_TOO)
🪛 Markdownlint
40-40: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
40-40: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
doc/assets/perception/Carla_Segmentation_Sensor.png
is excluded by!**/*.png
📒 Files selected for processing (1)
doc/research/paf24/segmentation.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/segmentation.md
[grammar] ~9-~9: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ... Carla Sensors - Follow up Question ## Al...
(CA_FOLLOW_UP)
[grammar] ~40-~40: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...n walks will be labeld as roadlines ## Follow up Question: Why the last group used bound...
(CA_FOLLOW_UP)
[uncategorized] ~41-~41: Possible missing comma found.
Context: ...bounding boxes and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~41-~41: “to” seems less likely than “too” (as well, very, ‘too much’).
Context: ...es and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_CP_TO_TOO)
🪛 Markdownlint
doc/research/paf24/segmentation.md
18-18: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
21-21: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
40-40: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
18-18: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
21-21: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
40-40: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
23-23: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
31-31: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
13-13: null
Bare URL used
(MD034, no-bare-urls)
16-16: null
Bare URL used
(MD034, no-bare-urls)
19-19: null
Bare URL used
(MD034, no-bare-urls)
22-22: null
Bare URL used
(MD034, no-bare-urls)
35-35: null
Bare URL used
(MD034, no-bare-urls)
23-23: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
doc/research/paf24/segmentation.md
Outdated
## Carla Sensors: | ||
https://carla.readthedocs.io/en/0.8.4/cameras_and_sensors/ | ||
``` | ||
camera = carla.sensor.Camera('MyCamera', PostProcessing='SemanticSegmentation') | ||
camera.set(FOV=90.0) | ||
camera.set_image_size(800, 600) | ||
camera.set_position(x=0.30, y=0, z=1.30) | ||
camera.set_rotation(pitch=0, yaw=0, roll=0) | ||
|
||
carla_settings.add_sensor(camera) | ||
``` | ||
![Alt text](https://github.com/una-auxme/paf/blob/368-visionnode-and-segmentation/doc/assets/perception/Carla_Segmentation_Sensor.png) | ||
|
||
there is another solution implemented by the carla simulator: | ||
https://carla.readthedocs.io/en/0.9.14/ref_sensors/#semantic-segmentation-camera:~:text=the%20object%20it.-,Semantic%20segmentation%20camera,-Blueprint%3A%20sensor | ||
|
||
for more context: | ||
the pedestrian walks will be labeld as roadlines | ||
|
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.
Update Carla sensor documentation and code example.
- The code example and documentation link are from Carla 0.8.4, which is outdated
- The image link should use a relative path
- There's a typo in "labeld" (line 38)
Update the code example to match current Carla version (0.9.14):
# Set up semantic segmentation camera
camera_bp = world.get_blueprint_library().find('sensor.camera.semantic_segmentation')
camera_bp.set_attribute('image_size_x', '800')
camera_bp.set_attribute('image_size_y', '600')
camera_bp.set_attribute('fov', '90.0')
# Add camera to vehicle
camera_transform = carla.Transform(carla.Location(x=0.30, y=0.0, z=1.30))
camera = world.spawn_actor(camera_bp, camera_transform, attach_to=vehicle)
Also, update the documentation link to the current version:
-https://carla.readthedocs.io/en/0.8.4/cameras_and_sensors/
+https://carla.readthedocs.io/en/0.9.14/ref_sensors/#semantic-segmentation-camera
And use relative path for the image:
-![Alt text](https://github.com/una-auxme/paf/blob/368-visionnode-and-segmentation/doc/assets/perception/Carla_Segmentation_Sensor.png)
+![Carla Segmentation Sensor](../../assets/perception/Carla_Segmentation_Sensor.png)
🧰 Tools
🪛 Markdownlint
21-21: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
21-21: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
23-23: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
31-31: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
22-22: null
Bare URL used
(MD034, no-bare-urls)
35-35: null
Bare URL used
(MD034, no-bare-urls)
23-23: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
doc/research/paf24/segmentation.md
Outdated
- [Segmentation](#Segmentation) | ||
- [Already implemended solutions](#1.-Already-implemented-solutions) | ||
- [Implemented but dropped](#2Implemented-but-dropped) | ||
- [Carla Sensors](#3-Carla-Sensors) | ||
- [Follow up Question](#4-Follow-up-Question) |
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.
Fix broken table of contents links.
The anchor links in the table of contents don't match the actual heading formats, which will result in broken navigation.
Apply this diff to fix the navigation:
-- [Already implemended solutions](#1.-Already-implemented-solutions)
-- [Implemented but dropped](#2Implemented-but-dropped)
-- [Carla Sensors](#3-Carla-Sensors)
-- [Follow up Question](#4-Follow-up-Question)
+- [Already implemented solutions](#already-implemented-solutions)
+- [Implemented but dropped](#implemented-but-dropped)
+- [Carla Sensors](#carla-sensors)
+- [Follow-up Question](#follow-up-question)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- [Segmentation](#Segmentation) | |
- [Already implemended solutions](#1.-Already-implemented-solutions) | |
- [Implemented but dropped](#2Implemented-but-dropped) | |
- [Carla Sensors](#3-Carla-Sensors) | |
- [Follow up Question](#4-Follow-up-Question) | |
- [Segmentation](#Segmentation) | |
- [Already implemented solutions](#already-implemented-solutions) | |
- [Implemented but dropped](#implemented-but-dropped) | |
- [Carla Sensors](#carla-sensors) | |
- [Follow-up Question](#follow-up-question) |
🧰 Tools
🪛 LanguageTool
[grammar] ~9-~9: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ... Carla Sensors - Follow up Question ## Al...
(CA_FOLLOW_UP)
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
doc/research/paf24/segmentation.md (4)
13-16
: Enhance documentation of implemented solutions.The section would be more helpful with:
- Brief descriptions of each implemented solution
- Properly formatted markdown links instead of raw URLs
- Explanation of how the dataset was generated and used
Example format:
## Already implemented solutions ### Current Implementation The current perception system uses [semantic segmentation launch configuration](link) which was trained using a [custom dataset generator](link). Key features: - Description of what the implementation does - How the dataset was generated - Performance characteristics🧰 Tools
🪛 Markdownlint
13-13: null
Bare URL used(MD034, no-bare-urls)
16-16: null
Bare URL used(MD034, no-bare-urls)
18-20
: Document reasons for dropping EfficientPS implementation.Please provide context about:
- Why was EfficientPS initially chosen?
- What issues or limitations led to dropping it?
- Lessons learned from this implementation attempt
This information would be valuable for future decision-making.
🧰 Tools
🪛 Markdownlint
18-18: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
18-18: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
19-19: null
Bare URL used(MD034, no-bare-urls)
29-45
: Improve code block formatting and documentation.
- Specify the language for the code block (python)
- Add comments explaining the key parameters:
- Image size choice (1920x1080)
- FOV setting (105)
- Camera position and rotation values
- Document the CityScapesPalette color mapping
-``` +```python🧰 Tools
🪛 Markdownlint
29-29: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
29-29: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
45-45: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
29-29: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
51-52
: Expand the architectural decision discussion.This is an important question that deserves thorough investigation. Consider:
- Benchmarking both approaches (bounding boxes vs segmentation)
- Documenting performance metrics
- Analyzing trade-offs in terms of:
- Processing speed
- Accuracy
- Resource usage
- Implementation complexity
Would you like help creating a comparison framework to evaluate both approaches?
🧰 Tools
🪛 LanguageTool
[grammar] ~51-~51: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...mera,-The%20semantic%20segmentation ## Follow up Question: Why the last group used bound...(CA_FOLLOW_UP)
🪛 Markdownlint
51-51: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
51-51: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/segmentation.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/segmentation.md
[grammar] ~51-~51: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...mera,-The%20semantic%20segmentation ## Follow up Question: Why the last group used bound...
(CA_FOLLOW_UP)
🪛 Markdownlint
doc/research/paf24/segmentation.md
29-29: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
18-18: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
21-21: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
51-51: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
18-18: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
21-21: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
51-51: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
29-29: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
45-45: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
13-13: null
Bare URL used
(MD034, no-bare-urls)
16-16: null
Bare URL used
(MD034, no-bare-urls)
19-19: null
Bare URL used
(MD034, no-bare-urls)
47-47: null
Bare URL used
(MD034, no-bare-urls)
49-49: null
Bare URL used
(MD034, no-bare-urls)
29-29: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
doc/research/paf24/segmentation.md (1)
25-27
: Clarify pedestrian crossing labeling details.
The note about pedestrian walks being labeled as roadlines needs more context:
- Is this a Carla limitation or a design choice?
- What are the implications for detection accuracy?
- Are there any workarounds or handling strategies?
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
doc/research/paf24/segmentation.md (4)
12-18
: Enhance documentation clarity in "Already implemented solutions" section.The section needs more context and explanation:
- Add descriptions of what these solutions actually implement
- Explain how the dataset generator relates to the implementation
- Use proper markdown link syntax instead of bare URLs
Example improvement:
## Already implemented solutions The perception stack currently implements segmentation using [this approach](link). This implementation relies on a model that was trained using our [custom dataset generator](link), which creates labeled data for the following classes: - [list the classes] The training process and model architecture are documented in...🧰 Tools
🪛 Markdownlint
14-14: null
Bare URL used(MD034, no-bare-urls)
17-17: null
Bare URL used(MD034, no-bare-urls)
19-22
: Document reasons for dropping EfficientPS implementation.To prevent future teams from repeating the same exploration, please document:
- Why was EfficientPS considered initially?
- What issues or limitations were encountered?
- What were the key factors in the decision to drop it?
🧰 Tools
🪛 Markdownlint
19-19: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
21-21: null
Bare URL used(MD034, no-bare-urls)
33-50
: Improve code example documentation and formatting.
- Add Python language specification to the code block
- Document the key parameters:
- Image size choice (1920x1080)
- FOV value (105)
- Camera position and rotation
- Explain the CityScapesPalette color converter
-``` +```python # Camera configuration for semantic segmentation # Resolution: 1920x1080 (Full HD) for detailed segmentation # FOV: 105 degrees to match our requirements # Position: 2m forward, 1m up for optimal view🧰 Tools
🪛 Markdownlint
33-33: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
57-59
: Improve follow-up question clarity and grammar.The question raises an important architectural consideration but needs revision:
-## Follow up Question: - -Why the last group used bounding boxes and not the segmentation model is it to slow or maybe not reliable? +## Follow-up Question: + +Why did the previous team choose bounding boxes over segmentation models? Was it due to performance concerns or reliability issues?Consider adding:
- Performance benchmarks comparing both approaches
- Reliability metrics if available
- Trade-offs between the two approaches
🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...mera,-The%20semantic%20segmentation ## Follow up Question: Why the last group used boun...(CA_FOLLOW_UP)
[uncategorized] ~59-~59: Possible missing comma found.
Context: ...bounding boxes and not the segmentation model is it to slow or maybe not reliable?(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~59-~59: “to” seems less likely than “too” (as well, very, ‘too much’).
Context: ...es and not the segmentation model is it to slow or maybe not reliable?(AI_HYDRA_LEO_CP_TO_TOO)
🪛 Markdownlint
57-57: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/segmentation.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/segmentation.md
[grammar] ~57-~57: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...mera,-The%20semantic%20segmentation ## Follow up Question: Why the last group used boun...
(CA_FOLLOW_UP)
[uncategorized] ~59-~59: Possible missing comma found.
Context: ...bounding boxes and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~59-~59: “to” seems less likely than “too” (as well, very, ‘too much’).
Context: ...es and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_CP_TO_TOO)
🪛 Markdownlint
doc/research/paf24/segmentation.md
19-19: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
23-23: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
57-57: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
14-14: null
Bare URL used
(MD034, no-bare-urls)
17-17: null
Bare URL used
(MD034, no-bare-urls)
21-21: null
Bare URL used
(MD034, no-bare-urls)
53-53: null
Bare URL used
(MD034, no-bare-urls)
55-55: null
Bare URL used
(MD034, no-bare-urls)
33-33: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
doc/research/paf24/segmentation.md (1)
28-30
: Clarify pedestrian crossing labeling information.
The statement about pedestrian walks being labeled as roadlines needs more context:
- Is this a limitation of Carla's segmentation?
- What implications does this have for our use case?
- Are there any workarounds available?
✅ Verification successful
Pedestrian crossing labeling is a known CARLA semantic segmentation limitation
Based on the documentation links and code context found, this is a known limitation of CARLA's semantic segmentation camera. The statement is documenting a technical constraint where pedestrian crossings/walks are labeled as part of the road lines in CARLA's segmentation system. This is important information for developers to be aware of when working with the segmentation data.
- The code shows proper setup of CARLA's semantic segmentation camera using the CityScapesPalette
- The limitation is documented alongside the camera configuration code
- This behavior is consistent with CARLA's semantic segmentation implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing handling of this limitation in the codebase
rg -i "pedestrian.*cross(ing|walk)" --type py
Length of output: 47
Script:
#!/bin/bash
# Let's broaden the search to find any mentions of pedestrian or road line segmentation
rg -i "(pedestrian|roadline|segmentation)" doc/research/paf24/ -A 3 -B 3
Length of output: 6333
Script:
#!/bin/bash
# Also check for any CARLA segmentation documentation or configuration files
fd -e py -e md -e json -e yaml -e yml . | xargs rg -i "(carla.*segment|segment.*carla)" -A 3 -B 3
Length of output: 4133
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
doc/research/paf24/segmentation.md (2)
1-11
: Add blank lines around headings for better readability.The document structure is good, but some headings need proper spacing for better readability.
Apply this diff:
# Segmentation **Summary:** This page contains the research into the segmentation component of Autoware. - [Segmentation](#Segmentation) - [Already implemented solutions](#already-implemented-solutions) - [Implemented but dropped](#implemented-but-dropped) - [Carla Sensors](#carla-sensors) - [Follow-up Question](#follow-up-question)
23-56
: Improve Carla sensor documentation presentation.
- Use relative paths for images
- Add language identifier to code block
- Simplify documentation links
- Add explanation about the CityScapesPalette color mapping
Apply these changes:
-![Alt text](https://carla.readthedocs.io/en/0.9.14/img/ref_sensors_semantic.jpg) -![Alt text](https://carla.readthedocs.io/en/0.9.14/img/tuto_sem.jpg) +![Semantic Segmentation Sensor](../../assets/carla/ref_sensors_semantic.jpg) +![Segmentation Example](../../assets/carla/tuto_sem.jpg) example: -```Python +```python # -------------- # Add a new semantic segmentation camera to my ego # --------------Also, simplify the documentation links:
-<https://carla.readthedocs.io/en/0.9.14/ref_sensors/#semantic-segmentation-camera:~:text=the%20object%20it.-,Semantic%20segmentation%20camera,-Blueprint%3A%20sensor> +[Carla Semantic Segmentation Camera Documentation](https://carla.readthedocs.io/en/0.9.14/ref_sensors/#semantic-segmentation-camera)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/research/paf24/segmentation.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/research/paf24/segmentation.md
[grammar] ~57-~57: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...era,-The%20semantic%20segmentation> ## Follow up Question Why the last group used bound...
(CA_FOLLOW_UP)
[uncategorized] ~59-~59: Possible missing comma found.
Context: ...bounding boxes and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~59-~59: “to” seems less likely than “too” (as well, very, ‘too much’).
Context: ...es and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_CP_TO_TOO)
🔇 Additional comments (1)
doc/research/paf24/segmentation.md (1)
12-22
:
Enhance documentation with more context and explicit findings.
The research documentation should explicitly address the questions from issue #368:
- Does Carla output segmentation data for:
- Lines
- Street signs
- Pedestrian crosswalks
- Are there pre-trained models available?
Please expand these sections with:
- Explanations of the linked code and its relevance
- Why certain solutions were dropped
- Clear answers to the original research questions
Let's verify the linked code still exists:
✅ Verification successful
Review comment is appropriate and the linked code exists
Based on the verification results:
- The linked code exists and is accessible
- The code confirms that:
- Carla does output segmentation data through its instance segmentation camera (verified in dataset_generator.py)
- The perception.launch file shows active image segmentation capabilities with models like "deeplabv3_resnet101" and "yolov8x-seg"
- EfficientPS was previously used but was dropped after paf22 (confirmed in efficientps.md)
- The documentation structure matches the research questions from issue VisionNode and Segmentation #368
The request for more context and explicit findings is valid because:
- The current documentation doesn't clearly answer whether specific features (lines, street signs, crosswalks) are included in the segmentation output
- The pre-trained model availability isn't explicitly addressed
- The reasons for dropping EfficientPS aren't explained
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the referenced code still exists
gh api repos/una-auxme/paf/commits/c3011ee70039e199e106c54aa162a8f52be241a6 > /dev/null
gh api repos/una-auxme/paf/commits/8e8f9a1a03ae09d5ac763c1a11b398fc1ce144b0 > /dev/null
gh api repos/una-auxme/paf/commits/8c968fb5c6c44c15b2733c5a181c496eb9b244be > /dev/null
Length of output: 233
Script:
#!/bin/bash
# Check if the referenced files exist and show their content
echo "=== Checking perception.launch ==="
fd "perception.launch" --exec cat {} || echo "File not found"
echo -e "\n=== Checking dataset_generator.py ==="
fd "dataset_generator.py" --exec cat {} || echo "File not found"
echo -e "\n=== Checking efficientps.md ==="
fd "efficientps.md" --exec cat {} || echo "File not found"
echo -e "\n=== Checking issue #368 ==="
gh issue view 368 2>/dev/null || echo "Issue not accessible"
Length of output: 17090
## Follow up Question | ||
|
||
Why the last group used bounding boxes and not the segmentation model is it to slow or maybe not reliable? |
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.
Improve grammar and clarity in the follow-up question.
The question needs better formatting and grammar.
Apply this diff:
-## Follow up Question
+## Follow-up Question
-Why the last group used bounding boxes and not the segmentation model is it to slow or maybe not reliable?
+Why did the previous group use bounding boxes instead of the segmentation model? Was it too slow or perhaps not reliable enough?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
## Follow up Question | |
Why the last group used bounding boxes and not the segmentation model is it to slow or maybe not reliable? | |
## Follow-up Question | |
Why did the previous group use bounding boxes instead of the segmentation model? Was it too slow or perhaps not reliable enough? |
🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: ‘Follow up’ seems to be a compound adjective before a noun. Use a hyphen: “Follow-up”.
Context: ...era,-The%20semantic%20segmentation> ## Follow up Question Why the last group used bound...
(CA_FOLLOW_UP)
[uncategorized] ~59-~59: Possible missing comma found.
Context: ...bounding boxes and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~59-~59: “to” seems less likely than “too” (as well, very, ‘too much’).
Context: ...es and not the segmentation model is it to slow or maybe not reliable?
(AI_HYDRA_LEO_CP_TO_TOO)
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.
looks good to me
Description
Research about the segmentation part in the visionnode
Fixes #368
Type of change
Create a new .md file where the research is summarized
Does this PR introduce a breaking change?
no
Most important changes
add new file to /doc/research/paf24/
Checklist:
Summary by CodeRabbit