Fix config option to `split_by_inds` #1060

roomrys · 2022-11-23T22:33:19Z

Description

Fix config option to split_by_inds. Previously triaining would fail when "split_by_inds": true due to improper handling of training, test, and validation indices. This PR converts the test, train, and validation sets to the format they are expected to be in Labels instead of List[LabeledFrame].

Types of changes

Does this address any currently open issues?

Training: config option to split_by_inds is broken #1059

Outside contributors checklist

Review the guidelines for contributing to this repository
Read and sign the CLA and add yourself to the authors list
Make sure you are making a pull request against the develop branch (not main). Also you should start your branch off develop
Add tests that prove your fix is effective or that your feature works
Add necessary documentation (if appropriate)

Thank you for contributing to SLEAP!

❤️

codecov · 2022-11-23T22:55:32Z

Codecov Report

Merging #1060 (770cb6f) into develop (846ce93) will increase coverage by 0.04%.
The diff coverage is 73.68%.

@@             Coverage Diff             @@
##           develop    #1060      +/-   ##
===========================================
+ Coverage    69.07%   69.12%   +0.04%     
===========================================
  Files          127      127              
  Lines        21599    21601       +2     
===========================================
+ Hits         14919    14931      +12     
+ Misses        6680     6670      -10

Impacted Files	Coverage Δ
sleap/nn/training.py	`60.78% <66.66%> (+1.28%)`	⬆️
sleap/nn/data/training.py	`51.76% <85.71%> (+0.57%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

roomrys · 2022-11-24T01:08:28Z

sleap/nn/training.py

@@ -20,6 +20,7 @@
 import copy

 import sleap
+from sleap import Labels


Patch coverage is down because I changed all sleap.Labels with Labels. My feature is tested and uses Labels so this shouldn't be a problem.

talmo

See inline comments about using Labels.extract instead of direct indexing. It might not be strictly necessary, but it would be beneficial while we're touching this part of the code.

If it doesn't work or if you see other issues with doing so, we can just go ahead and merge this as-is.

talmo · 2022-11-24T01:23:49Z

sleap/nn/training.py

@@ -177,14 +178,13 @@ def from_labels(
                    "Creating validation split from explicit indices "
                    f"(n = {len(labels_config.validation_inds)})."
                )
-                validation = training[labels_config.validation_inds]
-
+                validation = Labels(training[labels_config.validation_inds])


Here and in subsequent places, do we maybe want to switch to using Labels.extract()?
(In this case training.extract(labels_config.validation_inds, copy=False))

This retains other metadata which would be useful. We can skip using the copy=True since that might slow things down for really large datasets, but at least using extract() will avoid some weird issues with indexing videos and etc.

We may also want to switch to extract() here in sleap.nn.data.training.split_labels_train_val() for consistency:

sleap/sleap/nn/data/training.py

Lines 50 to 59 in 5093f69

# Create labels and keep original metadata.

labels_train = sleap.Labels(labels[idx_train])

labels_train.videos = labels.videos

labels_train.tracks = labels.tracks

labels_train.provenance = labels.provenance

labels_val = sleap.Labels(labels[idx_val])

labels_val.videos = labels.videos

labels_val.tracks = labels.tracks

labels_val.provenance = labels.provenance

roomrys

Some annotations for where the 3 functional changes are

roomrys · 2022-11-24T01:53:40Z

sleap/nn/data/training.py

-from sleap.nn.data.utils import expand_to_rank, ensure_list
+from sleap.nn.data.utils import ensure_list
 import attr
-from typing import List, Text, Optional, Any, Union, Dict, Tuple, Sequence
+from typing import List, Text, Dict, Tuple, Sequence


I removed unnecessary imports

roomrys · 2022-11-24T01:54:27Z

sleap/nn/data/training.py

@@ -3,20 +3,21 @@
 import numpy as np
 import tensorflow as tf
 import sleap
+from sleap import Labels


I replaced sleap.Labels with Labels

roomrys · 2022-11-24T01:55:19Z

sleap/nn/data/training.py

-    labels_train = sleap.Labels(labels[idx_train])
+    labels_train = labels.extract(idx_train, copy=False)
    labels_train.videos = labels.videos
    labels_train.tracks = labels.tracks
    labels_train.provenance = labels.provenance

-    labels_val = sleap.Labels(labels[idx_val])
+    labels_val = labels.extract(idx_val, copy=False)
    labels_val.videos = labels.videos
    labels_val.tracks = labels.tracks
    labels_val.provenance = labels.provenance


Functional change 1

roomrys · 2022-11-24T01:55:44Z

sleap/nn/data/training.py

-        split_labels.append(sleap.Labels([labels[int(ind)] for ind in sampled_indices]))
+        split_labels.append(
+            labels.extract([labels[int(ind)] for ind in sampled_indices])
+        )


Functional change 2

roomrys · 2022-11-24T01:57:33Z

sleap/nn/training.py

-                validation = training[labels_config.validation_inds]
-
+                validation = training.extract(labels_config.validation_inds, copy=False)
            if labels_config.test_inds is not None and len(labels_config.test_inds) > 0:
                logger.info(
                    "Creating test split from explicit indices "
                    f"(n = {len(labels_config.test_inds)})."
                )
-                test = training[labels_config.test_inds]
+                test = training.extract(labels_config.test_inds, copy=False)


Functional change 3a

roomrys · 2022-11-24T01:57:45Z

sleap/nn/training.py

-                training = training[labels_config.training_inds]
+                training = training.extract(labels_config.training_inds, copy=False)


Functional change 3b

talmo

lgtm

* GUI Training: Use hidden params from loaded config (#1053) * Add optional unragging arg to model export (#1054) * Fix config option to `split_by_inds` (#1060) * Convert training, validation, and test to Labels object * Add test for split_by_inds * Use Labels.extract instead of Labels(List[LabeledFrames]) * Tracking: robust assignment of the best score to an instance (#1062) * Set max instances for top down models (#1070) * Add optional unragging arg to model export * Add option to set max instances for multi-instance models * Fix test * Don't create instances during inference if no points were found (#1073) * Don't create instances during inference if no points were found * Add points check for all predictors * Fix single instance predictor logic and test * Add tests for all predictors Co-authored-by: roomrys <[email protected]> * Add one-line fix to VideoWriterSkyvideo (#1082) * Fix parser for sleap-export (#1085) * Refactor commands to load project as `AppCommand`s (#1098) * Add working Proof of Concept * Create command class for loading project * Split `LoadProjectFile` as a subclass of `LoadLabelsObject` * Reroute last existing reference * Remove debugging code * Flexibly resize input layer of `tf.keras.Model` upon loading trained model (#1084) * Add initial implementation (auto output stride problematic) * Add to load_predictor test (error when auto-compute output stride) * Use output stride from config instead of auto-computing * Fix output-stride/padding modulo error and do not resize on export * Fix resizing bug in multi-class predictors * Non-functional clean-up * Rename new input layer to original name * Add inference integration test * Minimize config surgery, generalize layer iteration Co-authored-by: Talmo Pereira <[email protected]> * Add Option to Make Trail Shade Darker/Lighter (#1103) * Make trails 60% darker than track color * Add menu option for shade of trails * Remove unexpected indent (fat-fingered) * Create signal that updates plot instead of removing and replotting items (#1134) * Create signal that updates plot instead of redrawing * Remove debug code * Non-functional self-review changes * Fix symmetric skeletons (via table input) (#1136) Ensure variable initialized before calling it * Nix export of tracking results (#1068) * [io] export tracking results to NIX file * [io] nix added to export filter only if available * [nixio] refactor, add scores link data as mtag * [nixio] speeding up export by chunked writing * [nixio] rename point score to node score * [nixio] fix missing dimension descriptor for node scores * [export analysis] support multiple formats also for bulk export * [nixio] export centroid, some documentation * [nixio] fix double dot before filename suffix * [nixio] fix bug when not all nodes were found * [nixio] housekeeping * [nix] add nix analyis output format to convert * [nix] tiny fix, catch file write error and properly close file * [inference] main takes optional args. Can be imported to run inference form scripts * [convert] simplify if else structure and outfile handling for analysis export * [nix] use pathlib instead of os * [nix] catch if there are instances with a None frame_idx ... not sure why this occurred. The nix adaptor cannot save instances that are not related to a frame. * [nix] move checks to top of write function * [nix] use absolute imports * [nix] use black to reformat * [commands] revert qtpy import and apply code style * [convert] use absolute imports, apply code style * [commands]fix imports * [inference/nix]fix linter complaint, adjust nix types for scores * [nix] add test case for nix export format * [nix] extended testing, some modifications of adaptor * [skeleton] add __eq__ to Skeleton ... make Node.name and Node.weight instance variables instead of class variables * [nix] add nixio to requirements, remove unused nix_available, ... allow for non-unique entries in node, track and skeleton. Extend node map to store the skeleton it is part of * [nix] make the linter happy * [Node] force definition of a name Co-authored-by: Liezl Maree <[email protected]> * [nix] use getattr for getting grayscale information Co-authored-by: Liezl Maree <[email protected]> * [nix] fixes according to review * [convert] break out of loop upon finding the video Co-authored-by: Liezl Maree <[email protected]> * [commands.py] use pathilb instead of splitting filename Co-authored-by: Liezl Maree <[email protected]> * [dev requirements] remove linebreak at last line * [skeleton] revert attribute creation back to original * [nix] break lines in class documentation * Ensure all file references are closed * Make the linter happy * Add tests for ExportAnalysis and (docs for) sleap-convert Co-authored-by: Liezl Maree <[email protected]> * Fix body vs symmetry subgraph filtering (#1142) Co-authored-by: Liezl Maree <[email protected]> * Handle changing backbones in training editor GUI (#1140) Co-authored-by: Liezl Maree <[email protected]> * Added scaling functionality for both the instances and bounding box. (#1133) * Create VisibleBoundingBox class. * Added instance scaling functionality in addition to bounding box scaling functionality. * Update sleap/gui/widgets/video.py Co-authored-by: Talmo Pereira <[email protected]> * Update sleap/gui/widgets/video.py Co-authored-by: Talmo Pereira <[email protected]> * Update sleap/gui/widgets/video.py Co-authored-by: Talmo Pereira <[email protected]> * Update sleap/gui/widgets/video.py Co-authored-by: Talmo Pereira <[email protected]> * Update sleap/gui/widgets/video.py Co-authored-by: Talmo Pereira <[email protected]> * Added new testing for scaling operation and simplified VisibleBoundingBox class code. * Added type hinting to the scaling padding and removed erroneous bounding rect initialization. Co-authored-by: Talmo Pereira <[email protected]> Co-authored-by: Liezl Maree <[email protected]> * Add better error message for top down (#1121) * Add better error message for top down * Add test for error message * Raise different error, fix test * Hotfix for video save #1098 (#1148) * Add a hotfix for #1098 * WIP: Add test for detecting changes on load * Finialize change on load test * Remove unused imports * Skip test if on windows since files are being used in parallel * Add central padding to SizeMatcher (#1129) * add center padding to size matcher * add test for center padding * add ensure_float option to inference layer * reformat resizing and test_resizing * Remove redundant operation * Replace existing constants with fixtures --------- Co-authored-by: Liezl Maree <[email protected]> * Added MoveNet as an external model reference (#1141) * add center padding to size matcher * add test for center padding * add ensure_float option to inference layer * reformat resizing and test_resizing * add MoveNet as an external model inference * add the movenet to the from_model_paths * add tests * add comments to movenet predictor * add tensorflow_hub to the requirements.txt * modified default video path * resolved most of the comments except expanding the predictor * expanded Predictor.from_model_paths function to include any pre-trained models. * add test_load_model * added from_trained_models in class Predictor and modified test_load_model for it. * modified test_load_model to be more generalized. * moved pretrained model from Predictor.from_trained_model to Predictor.from_model_paths and added a test for it. * Fix Predictor.from_model_paths and tests * Rename load_movenet_model to make_model_movenet * minor clean-up * Remove redundant operation * Replace existing constants with fixtures * Handle loading movenet models via load_model API * Clean-up doc strings --------- Co-authored-by: Liezl Maree <[email protected]> * Resumable Training (#1130) * add resume training functionality * add testing function for resume training functionality * linting black * Resumable Training 2 - CLI Options (#1131) * add cli options for resumable training * add test for cli resume training * black linting for cli resumable training * simplify resumable checkpoint CLI fn to a single CLI arg (#1132) * simplify resumable checkpoint CLI fn to a single CLI arg * Adam/resumable training 3 (#1150) * correct path of labels_path for test_training * add resume training to gui * add train from scratch message * Add finishing touches to resumable training PR (#1150) (#1168) * Refactor/update 'use trained' and 'resume training' checkbox logic * Simplify checkbox logic and reset model field when resume training * Reset checkboxes upon changing config selection * Handle case for updating TrainingEditor when sender is not a checkbox * Add complete state space GUI test for checkboxes * Finish combobox test * Test that form is reset * Remove straggling TODO --------- Co-authored-by: roomrys <[email protected]> Co-authored-by: jimzers <[email protected]> --------- Co-authored-by: jimzers <[email protected]> Co-authored-by: Liezl Maree <[email protected]> * Return trainer from sleap-train and check that trainer configured correctly * Add CLI documentation for website --------- Co-authored-by: jimzers <[email protected]> Co-authored-by: Liezl Maree <[email protected]> * Small (final?) revisions and fix test * Revert changes to fixture --------- Co-authored-by: jimzers <[email protected]> Co-authored-by: Liezl Maree <[email protected]> * GenericTableModel/View improvements (#1163) * [dataviews] GenericTableModel/View improvements ... * GenericTableView got a new argument specifying whether the ellipsis for long cell content should be right (old behavior, default) or left useful for long content such as the filenames in the video table. * GenericTableView uses all the space that available to the table. * The model's data function returns the full cell content to be shown as tool tip text. * [gui/app] set the ellipsis to be on the left for long table contents --------- Co-authored-by: Liezl Maree <[email protected]> * Add Skeleton Templates (#1122) * Update docs: change 'M1' to 'Apple Silicon' (#1188) * Bump to 1.3.0a0 (#1189) --------- Co-authored-by: sheridana <[email protected]> Co-authored-by: getzze <[email protected]> Co-authored-by: Talmo Pereira <[email protected]> Co-authored-by: Jan Grewe <[email protected]> Co-authored-by: Sean Afshar <[email protected]> Co-authored-by: Jiaying Hsu <[email protected]> Co-authored-by: Adam Lee <[email protected]> Co-authored-by: jimzers <[email protected]> Co-authored-by: Jan Grewe <[email protected]> Co-authored-by: Aaditya Prasad <[email protected]>

Convert training, validation, and test to Labels object

784e0c4

Add test for split_by_inds

c404591

roomrys marked this pull request as ready for review November 24, 2022 01:06

roomrys commented Nov 24, 2022

View reviewed changes

roomrys requested a review from talmo November 24, 2022 01:08

talmo reviewed Nov 24, 2022

View reviewed changes

Use Labels.extract instead of Labels(List[LabeledFrames])

8b53bf6

roomrys commented Nov 24, 2022

View reviewed changes

roomrys requested a review from talmo November 24, 2022 06:14

talmo approved these changes Nov 27, 2022

View reviewed changes

Merge branch 'develop' into liezl/fix_split_by_inds

770cb6f

roomrys merged commit 19b9503 into develop Dec 1, 2022

roomrys deleted the liezl/fix_split_by_inds branch December 1, 2022 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix config option to `split_by_inds` #1060

Fix config option to `split_by_inds` #1060

roomrys commented Nov 23, 2022

codecov bot commented Nov 23, 2022 •

edited

Loading

roomrys Nov 24, 2022

talmo left a comment

talmo Nov 24, 2022 •

edited

Loading

roomrys left a comment

roomrys Nov 24, 2022

roomrys Nov 24, 2022

roomrys Nov 24, 2022

roomrys Nov 24, 2022

roomrys Nov 24, 2022

roomrys Nov 24, 2022

talmo left a comment

	# Create labels and keep original metadata.
	labels_train = sleap.Labels(labels[idx_train])
	labels_train.videos = labels.videos
	labels_train.tracks = labels.tracks
	labels_train.provenance = labels.provenance

	labels_val = sleap.Labels(labels[idx_val])
	labels_val.videos = labels.videos
	labels_val.tracks = labels.tracks
	labels_val.provenance = labels.provenance

		training = training[labels_config.training_inds]
		training = training.extract(labels_config.training_inds, copy=False)

Fix config option to split_by_inds #1060

Fix config option to split_by_inds #1060

Conversation

roomrys commented Nov 23, 2022

Description

Types of changes

Does this address any currently open issues?

Outside contributors checklist

Thank you for contributing to SLEAP!

codecov bot commented Nov 23, 2022 • edited Loading

Codecov Report

roomrys Nov 24, 2022

Choose a reason for hiding this comment

talmo left a comment

Choose a reason for hiding this comment

talmo Nov 24, 2022 • edited Loading

Choose a reason for hiding this comment

roomrys left a comment

Choose a reason for hiding this comment

roomrys Nov 24, 2022

Choose a reason for hiding this comment

roomrys Nov 24, 2022

Choose a reason for hiding this comment

roomrys Nov 24, 2022

Choose a reason for hiding this comment

roomrys Nov 24, 2022

Choose a reason for hiding this comment

roomrys Nov 24, 2022

Choose a reason for hiding this comment

roomrys Nov 24, 2022

Choose a reason for hiding this comment

talmo left a comment

Choose a reason for hiding this comment

Fix config option to `split_by_inds` #1060

Fix config option to `split_by_inds` #1060

codecov bot commented Nov 23, 2022 •

edited

Loading

talmo Nov 24, 2022 •

edited

Loading