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

Added unzip installation as base ubuntu dependency and tool tests fixes #454

Merged
merged 18 commits into from
Aug 24, 2023
Merged

Added unzip installation as base ubuntu dependency and tool tests fixes #454

merged 18 commits into from
Aug 24, 2023

Conversation

tsampazk
Copy link
Collaborator

@tsampazk tsampazk commented Aug 17, 2023 •
edited
Loading

Fixes:

  • Possible fix missing unzip dependency for install script #453.
  • Replaced bcolz with bcolz-zipline to fix broken installation
  • In tests_suite.yml fixed a call for pip install /tests/sources/requirements.txt which is an empty file to pip install /tests/requirements.txt which seems to be the correct one
  • More strict pillow version, added <10.0.0, which breaks calls to LINEAR from detectron2 (which itself sits at an older version, newer versions have fixed this)
  • Changed ultralytics requirement to ultralytics-yolov5 requirement in object_detection_2d requirements file

Created the branch off master since it seems develop and master are not yet synchronized after latest release, let me know if this needs changing.

@tsampazk tsampazk added test tools Test the toolkit methods dependencies Pull requests that update a dependency file labels Aug 17, 2023
@tsampazk tsampazk self-assigned this Aug 17, 2023
@passalis passalis removed the test tools Test the toolkit methods label Aug 18, 2023
@passalis passalis added the test tools Test the toolkit methods label Aug 18, 2023
@passalis
Copy link
Collaborator

I replaced bcolz with bcolz-zipline, which made the situation much better, but I still see some failures in yolov5 (library not found), nms (couldn't download pretrained model'), and single demo grasp (an error probably due to PIL attribute missing).

@passalis
Copy link
Collaborator

@vivinousi @charsyme @amirmehman21 Could you please check the failures? Thank you!

@charsyme
Copy link
Collaborator

@passalis I changed/renamed some files on FTP server regarding object_detection_2d/nms tools. Once #442 will be merged, these specific failures will be dissmised.

@tsampazk
Copy link
Collaborator Author

This thread seems to have some information regarding the "AttributeError: module 'PIL.Image' has no attribute 'LINEAR'" of single demo grasp. I am not sure whether we can indeed update detectron2 and what other issues it might bring with embedded devices etc. @Pavlos-Tosidis any thoughts?

Maybe we can force an older version of PIL that uses the deprecated "LINEAR"?

@tsampazk tsampazk added test tools Test the toolkit methods and removed test tools Test the toolkit methods labels Aug 22, 2023
@Pavlos-Tosidis
Copy link
Collaborator

TX2 is not supported by NVIDIA anymore, which means the JetPack for it is an older version which does not support Python3.8, and that causes many problems with various updated dependencies , like Pytorch. So the update on the embedded devices docker images are being implemented mostly manually, with some of the tools not being able to run.
To conclude, the update on the toolkit for Desktop use won't break anything on the current images of the embedded devices, but some of the tools being added might not work.

@tsampazk tsampazk added the test tools Test the toolkit methods label Aug 22, 2023
@tsampazk tsampazk added the test sources Run style checks label Aug 23, 2023
@tsampazk
Copy link
Collaborator Author

Alright after some messing about, all tests (apart from the nms tool which is fixed in another PR) pass successfully. As soon as everything finishes im going to convert this to ready for review to go forward.

I also got some ideas about improving the workflows, i will test them in a subsequent PR.

@tsampazk tsampazk marked this pull request as ready for review August 23, 2023 16:12
@tsampazk tsampazk changed the title Added unzip installation as base ubuntu dependency Added unzip installation as base ubuntu dependency and tool tests fixes Aug 23, 2023
passalis
passalis approved these changes Aug 24, 2023
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!

@Pavlos-Tosidis Pavlos-Tosidis self-requested a review August 24, 2023 06:34
Pavlos-Tosidis
Pavlos-Tosidis approved these changes Aug 24, 2023
View reviewed changes
Copy link
Collaborator

@Pavlos-Tosidis Pavlos-Tosidis left a comment

Choose a reason for hiding this comment

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

Nice work Kostas.

@tsampazk
Copy link
Collaborator Author

@passalis I changed/renamed some files on FTP server regarding object_detection_2d/nms tools. Once #442 will be merged, these specific failures will be dissmised.

Nms is expected to fail due to changed filenames in FTP, i will go ahead and merge this.

@tsampazk tsampazk merged commit fb0e1d7 into master Aug 24, 2023
85 of 87 checks passed
@tsampazk tsampazk deleted the fix-unzip-install branch August 24, 2023 06:59
This was referenced Aug 24, 2023
Closed
Merged
lucamarchionni pushed a commit to lucamarchionni/opendr that referenced this pull request Jun 10, 2024
@tsampazk @passalis
Added unzip installation as base ubuntu dependency and tool tests fix…
887b79f 
…es (opendr-eu#454)

* Added unzip installation as base ubuntu dep

* Added DISABLE_BCOLZ_AVX2

* Added DISABLE_BCOLZ_AVX2

* Test bcolz only instllation

* Update install.sh

* Change from bcolz to bcolz-zipline

* Update install.sh

* Revert changes

* Revert changes

* Trying older version of ultralytics

* Added some diagnostics in env activation and (probably) fixed a requirements file for tests

* Fixed typo

* List pip installation

* Revert changes, error is caused by incompatible dependencies

* Trying older ultralytics version

* Made pillow installation strict, LINEAR is deprecated on version 10

* Trying looser dependency of pillow, less than 10.0

* Swapped ultralytics with ultralytics-yolov5 which uses ultralytics as requirement

---------

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

@Pavlos-Tosidis Pavlos-Tosidis Pavlos-Tosidis 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

@tsampazk tsampazk

Labels
dependencies Pull requests that update a dependency file test sources Run style checks test tools Test the toolkit methods
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

missing unzip dependency for install script
4 participants
@tsampazk @passalis @charsyme @Pavlos-Tosidis