Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2914 +/- ##
==========================================
+ Coverage 78.67% 78.72% +0.04%
==========================================
Files 194 194
Lines 26518 26549 +31
==========================================
+ Hits 20864 20900 +36
+ Misses 5654 5649 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jsbroks
approved these changes
Nov 16, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
JIRAs:
The
wandb.Imageclass can contain bounding box and mask annotations as well as class definitions. Until now, the Python type system was annotating Images incorrectly, which forces the Weave type system to attempt to infer the class (and annotation types) from the image content itself. This is particularly bad for two reasons: 1) inferring types from the data slows Weave down and breaks the fundamental pattern that the graph can be operated on without inspecting the data; 2) if a table has an image column, it would need to inspect every single image to get a comprehensive type for the column. Currently, we have some Python rules that enforce a single annotation type, so in the Weave system, we just inspect the first Image to determine the type. This was always a buggy part of the system that we (I?) was delaying fixing - and in particular were relying on the stricter constraints that enforced homogenous columns. However, the Growth team (@AyushExel and @morganmcg1) found some compelling use cases which need different annotation class sets within the same column (for detectron2 integation). So, finally it comes time to get this right.First, let's understanding the high level data structure (annotated typescript style)
There are a number of odd parts of this:
.... uggg
Something else: individual boxes (the
BOX_DATA_TYPE) above can contain a dictionary of scores - which can be used to filter the boxes down.Ok, so the goal of this PR is to:
Imagelevel class object be fully representative of the entire set of classes below.Note: this PR should not effect any user scripts - we only loosen some constraints and add additional metadata to the artifact logging. There is another PR https://github.com/wandb/core/pull/8169 which will contain the frontend changes which can consume such additional type metadata. Note: these can be deployed in any order as the current UI should not be effected by this change, and the future UI still needs to work on the old CLI.
Testing
How was this PR tested?
Unit tests + manual tests:
Release Notes
Below, please enter user-facing release notes as one or more bullet points.
If your change is not user-visible, write
NO RELEASE NOTESinstead, with no bullet points.------------- BEGIN RELEASE NOTES ------------------
------------- END RELEASE NOTES --------------------