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

Fix 3D Detection And 2D Tracking Tests #207

Merged
merged 36 commits into from
Sep 7, 2022
Merged

Fix 3D Detection And 2D Tracking Tests #207

merged 36 commits into from
Sep 7, 2022

Conversation

iliiliiliili
Copy link
Collaborator

@iliiliiliili iliiliiliili commented Jan 31, 2022 •
edited
Loading

Fix for 3D object detection free pointer error

@iliiliiliili iliiliiliili added the test tools Test the toolkit methods label Jan 31, 2022
@iliiliiliili
Copy link
Collaborator Author

@passalis @ad-daniel It does not seem to appear in this PR, but I removed del learner lines that are most likely to cause this. It was used to reduce RAM load when all tests were done inside a single test group.

@iliiliiliili iliiliiliili marked this pull request as ready for review January 31, 2022 12:09
@iliiliiliili iliiliiliili added test sources Run style checks test packages Test pip wheel and docker image labels Jan 31, 2022
@iliiliiliili
Copy link
Collaborator Author

iliiliiliili commented Jan 31, 2022 •
edited
Loading

This error persists for 3D tracking tests too and most likely is a problem of g++ compilation and python bounding

@iliiliiliili iliiliiliili removed test sources Run style checks test tools Test the toolkit methods labels Jan 31, 2022
@ad-daniel ad-daniel marked this pull request as draft February 9, 2022 13:36
@ad-daniel ad-daniel added test release Tests if a wheel created from a branch runs correctly and removed test packages Test pip wheel and docker image labels Mar 4, 2022
iliiliiliili and others added 9 commits April 3, 2022 22:54
@iliiliiliili
Merge branch 'master' into fix-3d-tests
d96806d
@passalis @stefaniapedrazzi @ad-daniel
Docker, installation and documentation improvements (#276)
d4dec9a 
* docker fixes

* Style fixes

* Create customize.md

* Removed docker building instructions

* Update customize.md

* Update customize.md

* Update installation.md

* Update customize.md

* Added link for customization

* Update docs/reference/installation.md

Co-authored-by: Stefania Pedrazzi <[email protected]>

* Update docs/reference/installation.md

Co-authored-by: Stefania Pedrazzi <[email protected]>

* Update docs/reference/installation.md

Co-authored-by: ad-daniel <[email protected]>

Co-authored-by: Stefania Pedrazzi <[email protected]>
Co-authored-by: ad-daniel <[email protected]>
@vivinousi
DetectionDataset evaluation fix for empty detections (#285)
f106e30
@passalis
Checkout specific detectron commit
cba65ac
@passalis
Install specific detectron2 commit
f385858
@passalis
Point to specific detectron2 commit
94d7281
@passalis
Update installation.md
1cbde46
@passalis
Update dependencies.ini
5594d4b
@passalis
Update dependencies.ini
d884c2c
@iliiliiliili
Copy link
Collaborator Author

I have resolved a problem with slow data loader for KITTI and simplified testing models and tests, so the memory issue should have much fewer chances to occur.

@iliiliiliili iliiliiliili marked this pull request as ready for review August 23, 2022 21:59
@iliiliiliili iliiliiliili added test sources Run style checks test tools Test the toolkit methods test packages Test pip wheel and docker image and removed test packages Test pip wheel and docker image test release Tests if a wheel created from a branch runs correctly labels Aug 23, 2022
@passalis passalis mentioned this pull request Aug 26, 2022
Merged
@stefaniapedrazzi
Copy link
Collaborator

This PR contains many unrelated changes.
It may be that it is based on the master branch but targeting the develop branch.

If the test error is also reproducible on the master branch, then this PR should be merged to the master branch and not the develop branch.

@iliiliiliili iliiliiliili changed the base branch from develop to master August 31, 2022 10:17
@iliiliiliili iliiliiliili changed the title Fix 3D Detection Tests Fix 3D Detection And 2D Tracking Tests Aug 31, 2022
@ad-daniel ad-daniel added test release Tests if a wheel created from a branch runs correctly and removed test tools Test the toolkit methods labels Sep 5, 2022
@ad-daniel ad-daniel added test release Tests if a wheel created from a branch runs correctly and removed test release Tests if a wheel created from a branch runs correctly labels Sep 6, 2022
@ad-daniel
Copy link
Collaborator

I've run this version multiple times and it seems that both object_detection_3d and object_tracking_3d are working now, it hasn't run out of memory so far either, so the simplification seems effective.
If you think the current version of the test is representative of the tool and that what we test suffices to notify us if future changes (like changing dependencies to some other tool) breaks this one then I think that after a final cleanup (removing commented lines) we can proceed to merge the PR.

ad-daniel
ad-daniel approved these changes Sep 7, 2022
View reviewed changes
Copy link
Collaborator

@ad-daniel ad-daniel left a comment

Choose a reason for hiding this comment

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

Thank you!

passalis
passalis approved these changes Sep 7, 2022
View reviewed changes
Copy link
Collaborator

@passalis passalis left a comment

Choose a reason for hiding this comment

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

Thank you!

@passalis passalis merged commit d8b1572 into master Sep 7, 2022
@passalis passalis deleted the fix-3d-tests branch September 7, 2022 10:32
lucamarchionni pushed a commit to lucamarchionni/opendr that referenced this pull request Jun 10, 2024
@iliiliiliili @passalis
@stefaniapedrazzi @ad-daniel @vivinousi @tsampazk
Fix 3D Detection And 2D Tracking Tests (opendr-eu#207)
4711ff1 
* include 3d detection test

* fix np bool warning

* Revert "fix np bool warning"

This reverts commit ec21061.

* Remove del learner

* Eneable tracking 3d tests

* Add init for 2d tracking tests

* Update 3D tracking dependencies

* Docker, installation and documentation improvements  (opendr-eu#276)

* docker fixes

* Style fixes

* Create customize.md

* Removed docker building instructions

* Update customize.md

* Update customize.md

* Update installation.md

* Update customize.md

* Added link for customization

* Update docs/reference/installation.md

Co-authored-by: Stefania Pedrazzi <[email protected]>

* Update docs/reference/installation.md

Co-authored-by: Stefania Pedrazzi <[email protected]>

* Update docs/reference/installation.md

Co-authored-by: ad-daniel <[email protected]>

Co-authored-by: Stefania Pedrazzi <[email protected]>
Co-authored-by: ad-daniel <[email protected]>

* DetectionDataset evaluation fix for empty detections (opendr-eu#285)

* Checkout specific detectron commit

* Install specific detectron2 commit

* Point to specific detectron2 commit

* Update installation.md

* Update dependencies.ini

* Update dependencies.ini

* Update requirements.txt

* Update dependencies.ini

* Update dependencies.ini

* Update requirements.txt

* Update to support newer skimage version

* Clarified purpose and limitations of fall detection tool (opendr-eu#280)

* Enable All 3d test

* Fix slow data preparation for KITTI

* Simplify 3d detection tests

* Fix unclosed file

* Fix repeated eval failure

* Cleanup

* Cleanup

* Cleanup

* Remove unused test functions

Co-authored-by: Nikolaos Passalis <[email protected]>
Co-authored-by: Stefania Pedrazzi <[email protected]>
Co-authored-by: ad-daniel <[email protected]>
Co-authored-by: vivinousi <[email protected]>
Co-authored-by: Kostas Tsampazis <[email protected]>
Co-authored-by: ad-daniel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ad-daniel ad-daniel ad-daniel approved these changes

@passalis passalis passalis approved these changes

@omichel omichel Awaiting requested review from omichel omichel is a code owner

@stefaniapedrazzi stefaniapedrazzi Awaiting requested review from stefaniapedrazzi stefaniapedrazzi is a code owner

Assignees
No one assigned
Labels
test release Tests if a wheel created from a branch runs correctly test sources Run style checks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@iliiliiliili @stefaniapedrazzi @ad-daniel @passalis @vivinousi @tsampazk