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.

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

Beta2 rollout #323

Merged
merged 41 commits into from
Dec 24, 2020
Merged

Beta2 rollout #323

merged 41 commits into from
Dec 24, 2020

Conversation

beijbom
Copy link
Collaborator

@beijbom beijbom commented May 25, 2020

This is the base-branch for the new back-end rollout:

BONUS (can be pushed to later)

…serving (as far as could be manually tested dev-side).
…ready works (as far as could be manually tested dev-side).
…ross-platform (as far as could be manually tested dev-side).

Manually tested the basic behavior for everything except the classify task (which got an error calling predict_proba() on the model; not sure if a pickle cross-platform issue or something else).
beijbom and others added 4 commits May 25, 2020 14:10
* added spacer 0.2 to requirement. Changed to chardet (not cchardet) in local installs

* fixed int conversion -- skipped failing unit-test for now

* Lots of work remaining, but got the extract_features task to execute using the new messaging and spacer system.

* started working on train classifier task

* fixed get_secret for spacer job hash

* added force_dummy_extractor setting

* reworked submit_classifier. Still need to handle the previous classifiers

* fixed PEP 8 line breaks

* classify_image task done

* updated deploy tasks.

* updated calls to add_scores and add_annotations to use the new interface

* encapsulated the spacer_job_token encoding and decoding

* renamed the backend abstraction to queue abstraction, simplified unique filename by using microseconds

* removed FORCE_NO_BACKEND_SUBMIT

* fixed the renaming from backends to queues

* upgrade pyspacer version, added test-queue for dev-config

* bumped pyspacer one version to fix the aws permission error

* fixed import from spacer to avoid using the fire module

* implemented path method for s3 storage backend

* renamed factory method from backend to queue

* abstracted away more stuff to the storage backend, enabled unit-tests with spacer and the s3 storage

* removed more usages of storage.path

* updated to use spacer storage to store train and val data

* added more train_classifier tests

* Updated views.py to use new data-classes. Renamed config and classes related to LocalQueue

* removed the regtests configs, updated pyspacer req

* bumped pyspacer version to 0.2.6

* Updated vb_regtests.py

* removed outdated scripts.py

* added some more test for train_classifier

* removed hacky exists_full method on the storage classes. Removed redundant feature submit call

* made backend_overview page work even if celery is not running

* added full help text to regtests and more docstrings. Also sorted images to make sure the 'small' mode has at least 1 of each label

* fixed bug in tasks.reset_features(). Made the vb_regtests more robust

* One more safeguard agains raceconditions

* Changed to microseconds in comment.

* Update storage_backends.py

* Purged path_full

* Add migrations to change fields' string attributes from byte strings to Unicode strings. These were generated by `makemigrations` after switching to Python 3.

None of the attributes seem to contain non-ASCII characters, so the type conversion should not pose issues.

* Skip the images 0023 migration test because it now fails on Travis.

* RemovePointOutliersTest was unreliable to begin with on Windows dev environment, and now it seems unreliable on Travis too. Just skip for now.

* Update vb_regtests.py

Fixed typo in instructions.

Co-authored-by: StephenChan <StephenChan@users.noreply.github.com>
Co-authored-by: StephenChan <stephenjchan@gmail.com>
@beijbom
Copy link
Collaborator Author

beijbom commented Jun 18, 2020

@StephenChan : I merged the first PR. Do you want to take a stab next and address any combination of your 3 items in the PR description. Doesn't seem like the order matter that much. Once done I'll take one more pass on the vision_backend app.

@beijbom beijbom self-assigned this Jun 18, 2020
@StephenChan
Copy link
Member

Do you want to take a stab next and address any combination of your 3 items in the PR description. Doesn't seem like the order matter that much. Once done I'll take one more pass on the vision_backend app.

Sure. I can do 1) more vision backend tests and 3) support for different extractors. The second item, 'Purge python 2.7 compatibility code', doesn't necessarily have to happen before the rollout. Though I'll make sure to use Py3-only code for this branch.

@beijbom
Copy link
Collaborator Author

beijbom commented Jun 19, 2020 via email

@StephenChan
Copy link
Member

Great. Let’s test explicitly for duplicate row/col values. I didn’t handle that explicitly, so I’m curious if it works out anyways.

Agreed, there should be tests for that.

Also, a follow-up on the string-attribute related migrations from PR #324:

Just in case, I'll set up a Python 3 environment on our staging server and see if that encounters any issues with the new migrations.

I set up Python 3 on the staging server. The migration unit test fails, just like on Travis. However, the migration itself seems okay. I ran the images 0024 migration, and started the staging server. Didn't see any issues with existing metadata. Rolled it back to 0023, still no issues seen. So it's likely the unit test's issue.

* Shuffled such that py3 migration is the last one.

* Added some test settings overrides
* AWS batch integration
beijbom and others added 3 commits November 12, 2020 20:03
* Attempting to un-skip test_deploy.py's SuccessTest.test_done, but with LocalQueue it gets `TypeError: Object of type 'int32' is not JSON serializable`.

* bumped pyspacer version

* Fix the rest of test_deploy.SuccessTest.test_done.

* Unskip test_deploy.TaskErrorsTest.test_classifier_deleted. Add a test for handling spacer-side deploy errors.

* Unskip the last two skipped deploy tests.

* Improve explanation on skipping test_backend_overview.

* Fix TestDeployCollector; it was broken by a previous commit.

Co-authored-by: StephenChan <stephenjchan@gmail.com>
…ssifier use a mock Image.valset property, so that the image counts going into training vs. validation can be easily controlled.
…t class.

(Testing the classify-related util functions individually would still be worthwhile, but ClassifyUtilsTest was kind of in an odd middle ground, somewhere between single-function unit tests and full integration tests. Essentially, this commit pushes it to full integration tests.)
…olerant of any score values from deploy, since it seems the values can vary.
StephenChan and others added 16 commits December 3, 2020 18:32
…so that we can hopefully see what's going on whenever it decides to fail in Travis again (seems it may be an intermittent thing, and only happens in Travis).
More tests for vision backend tasks
- Existing sources get set to VGG16, and new sources default to EfficientNet.
- Allow the field to be set in the Create Source and Edit Source forms.
- Display the field value on the source main page.
…or that source. Warn the user appropriately.

- Move the Source Edit cancel button to a separate form, to make it easier to assign a submit handler to the main form.
- Rename the reset_after_labelset_change task to reset_backend_for_source.
… resets classifiers and classifications, and one that resets that plus features.

- Expand on tests involving these tasks.
- Refactor other VB tasks tests, mainly removing the need to decorate every BaseTaskTest subclass with mock.patch().
…Not a fix exactly, but remove unnecessary collect_all_jobs() calls.
Enable support for different feature extractors
@StephenChan StephenChan self-requested a review December 23, 2020 21:35
Copy link
Member

@StephenChan StephenChan left a comment

Choose a reason for hiding this comment

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

This looks finally ready to go, as far as I can tell.

@beijbom Any other loose ends you can think of, besides the announcement blog post?

@beijbom
Copy link
Collaborator Author

beijbom commented Dec 24, 2020

Very nice! Nope; this should be good to go!

@kriegman
Copy link
Collaborator

kriegman commented Dec 24, 2020 via email

@StephenChan StephenChan merged commit 5f7c6ad into master Dec 24, 2020
@StephenChan StephenChan deleted the beta2-rollout branch November 11, 2023 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants