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.

Sign up for GitHub

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

Jump to bottom

feat(perception): Use 2D bounding boxes for Occlusion Modifiers #343

Merged
merged 18 commits into from
Sep 12, 2023
Merged

feat(perception): Use 2D bounding boxes for Occlusion Modifiers #343

merged 18 commits into from
Sep 12, 2023

Conversation

schwepmo
Copy link
Contributor

@schwepmo schwepmo commented Aug 18, 2023 •
edited
Loading

Description

  • With the addition of vehicle dimension into the VehicleObject, we can now calculate the bounding boxes (for now only 2D)
  • The SpatialObjectBoundingBox interface has been added which now part of a SpatialObject and there exists two implementations:
    • PointBoundingBox -> for objects that shouldn't be treated as having a bounding box, their only "corner" will be the given position
    • VehicleBoundingBox -> for now used for vehicles, a rotated rectangle dependent on the heading of a vehicle.
  • The changes allow for more realistic occlusion evaluation

Below are some examples of changed behavior using vehicles from the unit test:

Vehicle Occlusion:

Old New>
#perceived vehicles 27 29
image
Quantitative there is no larger difference, with the bounding box based occlusion, actually a few more vehicles are perceived. What is noticeable, is that further non-occluded vehicles are now visible because the range penalty of the old modifier doesn't apply anymore

Wall Occlusion:

Old New>
#perceived vehicles 87 86
image
here the difference is actually quite small. But one edge case has been randomly caught, where a vehicle crosses a wall. This vehicle is no longer visible, as 3 of its' corners are actually behind the wall.

Issue(s) related to this PR

  • Resolves internal issue 630

Affected parts of the online documentation

  • Yes we will need to adjust some documentation related to perception

Definition of Done

Prerequisites

  • You have read CONTRIBUTING.md carefully.
  • You have signed the Contributor License Agreement.
  • Your GitHub user id is linked with your Eclipse Account.

Required

  • The title of this merge request follows the scheme type(scope): description (in the style of Conventional Commits)
  • You have assigned a suitable label to this pull request (e.g., enhancement, or bugfix)
  • origin/main has been merged into your Fork.
  • Coding guidelines have been followed (see CONTRIBUTING.md).
  • All checks on GitHub pass.
  • All tests on Jenkins pass.

Requested (can be enforced by maintainers)

  • New functionality is covered by unit tests or integration tests. Code coverage must not decrease.
  • If a bug has been fixed, a new unit test has been written (beforehand) to prove misbehavior
  • There are no new SpotBugs warnings.

Special notes to reviewer

@schwepmo
fix(perception): some fixes in perception model
9736332 
Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
feat(perception): initial, non-functional bounding box occlusion
3bad5e6 
Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
feat(perception): added bounding box occlusion modifier (needs to be …
ce70fc2 
…further tested)

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
feat(perception): added bounding box attribute to SpatialObject and p…
96dd40e 
…roperly implemented

* BoundingBoxOcclusionModifier now evaluates if points are hidden behind other bounding boxes
* WallOcclusionModifier now uses bounding boxes to see if objects are hidden

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo schwepmo added the enhancement New feature or request label Aug 18, 2023
@schwepmo schwepmo requested a review from kschrab August 18, 2023 13:48
kschrab
kschrab approved these changes Aug 29, 2023
View reviewed changes
Copy link
Contributor

@kschrab kschrab left a comment

Choose a reason for hiding this comment

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

some minor comments

...d/application/ambassador/simulation/perception/errormodels/BoundingBoxOcclusionModifier.java Outdated Show resolved Hide resolved
...d/application/ambassador/simulation/perception/errormodels/BoundingBoxOcclusionModifier.java Outdated Show resolved Hide resolved
...d/application/ambassador/simulation/perception/errormodels/BoundingBoxOcclusionModifier.java Outdated Show resolved Hide resolved
...d/application/ambassador/simulation/perception/errormodels/BoundingBoxOcclusionModifier.java Outdated Show resolved Hide resolved
...d/application/ambassador/simulation/perception/errormodels/BoundingBoxOcclusionModifier.java Outdated Show resolved Hide resolved
...osaic/fed/application/ambassador/simulation/perception/index/objects/TrafficLightObject.java Outdated Show resolved Hide resolved
...osaic/fed/application/ambassador/simulation/perception/index/objects/VehicleBoundingBox.java Show resolved Hide resolved
...osaic/fed/application/ambassador/simulation/perception/index/objects/VehicleBoundingBox.java Outdated Show resolved Hide resolved
...pse/mosaic/fed/application/ambassador/simulation/perception/index/objects/VehicleObject.java Outdated Show resolved Hide resolved
...osaic/fed/application/ambassador/simulation/perception/index/objects/VehicleBoundingBox.java Outdated Show resolved Hide resolved
@schwepmo
fix(perception): working on PR-review
1a2beb2 
*  renaming of occlusion modifiers

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
Merge branch 'main' of github.com:eclipse/mosaic into 630-bounding-bo…
66ef4ba 
…x-occlusion
@schwepmo
fix(perception): working on PR-review
1146d2e 
* improved a lot of documentation
* improved the place where bounding boxes are calculated
* extracted method in BoundingBoxOcclusion

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
fix(perception): working on PR-review
b5493fa 
* adjusted names of perception modifiers

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@kschrab
Copy link
Contributor

kschrab commented Sep 1, 2023

I just noticed that you have 2 new spotbugs warnings :<

@schwepmo
Copy link
Contributor Author

schwepmo commented Sep 4, 2023

I just noticed that you have 2 new spotbugs warnings :<

Shame on me! I will try to fix it ASAP.

@schwepmo
cleanup(mapping): fixed check style warnings
c602258 
Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
cleanup(apps): fixing spotbugs warning in example app
ab4e511 
Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
cleanup(perception): fixing spotbugs warning in perception objects
ee82c00 
Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
cleanup(perception): trying to fix spotbugs warning in perception obj…
188b531 
…ects

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
cleanup(perception): trying to fix spotbugs warning in perception obj…
b4e93a1 
…ects -> made VehicleIndex no longer implement Serializable

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
cleanup(example-apps): fixing spotbugs warning example app
a39a6e7 
Signed-off-by: Moritz Schweppenhäuser <[email protected]>
@schwepmo
cleanup(perception): tyring to spotbugs warnings in perception
9c5ce92 
* removed unnecessary method getNumberOfTrafficLights in TrafficObjectIndex

Signed-off-by: Moritz Schweppenhäuser <[email protected]>
kschrab
kschrab requested changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@kschrab kschrab left a comment

Choose a reason for hiding this comment

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

You also need to create an MR on Extended side.

...ipse/mosaic/fed/application/ambassador/simulation/perception/CentralPerceptionComponent.java Outdated Show resolved Hide resolved
...ipse/mosaic/fed/application/ambassador/simulation/perception/CentralPerceptionComponent.java Show resolved Hide resolved
@kschrab kschrab merged commit 0b1074d into main Sep 12, 2023
1 check passed
@kschrab kschrab deleted the 630-bounding-box-occlusion branch September 13, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kschrab kschrab kschrab approved these changes

Assignees

@kschrab kschrab

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@schwepmo @kschrab