Skip to content

Update library requirements.txt#27

Merged
SkepticRaven merged 18 commits into
mainfrom
update-reqs
Sep 20, 2023
Merged

Update library requirements.txt#27
SkepticRaven merged 18 commits into
mainfrom
update-reqs

Conversation

@SkepticRaven

Copy link
Copy Markdown
Contributor

Updating the requirements.txt to not be 2-3 year old libraries.
There were a couple other changes to get compatibility working again.

Tests run at the beginning of this pull:

  • Ran unittests: python3 -m unittest discover -s /behavior-classifier/tests/
    • Ran 33 tests in 0.785s OK
  • Ran python3 initialize_project.py on a project
  • Opened the GUI from an updated gui-vm and trained a classifier

I don't have a bunch of dev environments to test, so these were all conducted inside singularity (running debian).
One of the fixed I needed to do (upgrade to shapely change) may address #5 (comment)
Also address #24

Would be good to have someone with a mac environment to share their experience.

@SkepticRaven SkepticRaven requested review from anshu957 and dahhei July 20, 2023 19:52
@SkepticRaven SkepticRaven added the enhancement New feature or request label Jul 20, 2023
@SkepticRaven

Copy link
Copy Markdown
Contributor Author

Works successfully on windows 10 (python 3.10.11)

@SkepticRaven

Copy link
Copy Markdown
Contributor Author

@dahhei tried to install the venv on a mac and discovered shiboken needed to be updated from 5 -> 6

@anshu957

Copy link
Copy Markdown
Collaborator

The app gets installed without any issue on mac M1. I can also train a classifier for single mouse dataset. However, when I try to export the classifier, I get the following error:

Traceback (most recent call last):
  File "/Users/chouda/tmp/jabs_app/JABS-behavior-classifier/src/ui/main_window.py", line 215, in _export_training_data
    out_path = export_training_data(self._project,
  File "/Users/chouda/tmp/jabs_app/JABS-behavior-classifier/src/project/export_training.py", line 92, in export_training_data
    (1,), dtype=np.int)
  File "/Users/chouda/tmp/jabs_app/JABS-behavior-classifier/jabs.venv/lib/python3.9/site-packages/numpy/__init__.py", line 313, in __getattr__
    raise AttributeError(__former_attrs__[attr])
AttributeError: module 'numpy' has no attribute 'int'.
`np.int` was a deprecated alias for the builtin `int`. To avoid this error in existing code, use `int` by itself. Doing this will not modify any behavior and is safe. When replacing `np.int`, you may wish to use e.g. `np.int64` or `np.int32` to specify the precision. If you wish to review your current use, check the release note link for additional information.

@SkepticRaven

Copy link
Copy Markdown
Contributor Author

This branch would have particularly benefited from having more coverage on unittests.
Given that I missed that one, I should probably click all the buttons on the gui and even run tests on the non-gui version to make sure things don't change before a merge.
New list of tests that should probably go through:

  • Click all the buttons on gui
  • Test multiple project pose versions (2, 3, 4, 5)
  • Test command line interface (classify.py, initialize_project.py, and stats.py)

Also I upgraded the singularity gui vm from buster -> bookworm, so should make the non-gui one also be bookworm (not bullseye).

@dahhei dahhei left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The requirements.txt works without errors on my local M1 Macbook. Exporting a classifier also works fine on my end now.

@SkepticRaven

Copy link
Copy Markdown
Contributor Author

A couple notes from testing/poking around:

  • (Critical) Pause (after play) causes segfault ~50% of the time
    • Currently unclear cause, as threads seem to get closed correctly. It's happening after/during the repaint even after it stops
  • (Minor) Text littered around the UI is now lighter compared to older versions
    • This may be linked to OS settings and may be limited to my Ubuntu host machine (not present in CentOS test)

SkepticRaven added a commit that referenced this pull request Aug 1, 2023
@SkepticRaven

Copy link
Copy Markdown
Contributor Author

Latest commit should clear the 2 issues above.
Remaining checks before merge:

  • Confirm crash fix native on macs
  • Test stats.py

Conda environment was required for apple silicon but were reverted to reduce support complexity due to upgrade to pyside6 also resolving that.
See #5 (comment) for more info.
Classifiers use new xgboost/scikitlearn
Features use new numpy/shapely
Software version always gets a bump
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

xgboost doesn't use multiple threads for training / upgrading library versions

3 participants