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

Refactor pipelines into classes #51

Merged
merged 104 commits into from
Aug 17, 2023
Merged

Refactor pipelines into classes #51

merged 104 commits into from
Aug 17, 2023

Conversation

talmo
Copy link
Contributor

@talmo talmo commented Aug 17, 2023

This PR constitutes a massive refactor started in #45 and #50 to fix how we compute our traits in a pipeline form.

While there are many changes, the key differences are that we now have the trait computation graph fully defined and structured as a set of classes in sleap_roots.trait_pipelines. They are organized into:

  • TraitDef: This defines the concept of a trait, including which other traits are used to compute it, its name, the function that is used to compute it, and additional metadata such as whether it's scalar (i.e., needs to be summarized) and whether it should be included in output CSVs. This allows us to be flexible in defining intermediate traits that may not be included in final analyses but that are necessary to compute other traits. Fully defining the inputs and outputs also allows us to compute all of the traits in the appropriate order by enforcing a topological ordering of the computation graph.
  • Pipeline: This is a base class which implements the generic trait computation steps, including frame-level, plant-level, and batch-level steps. Subclasses of Pipeline (such as DicotPipeline) can be defined by just inheriting from this class and defining two functions:
    • define_traits: Returns a list of TraitDef that defines the set of traits computed by the pipeline.
    • get_initial_frame_traits: Returns a dictionary of initial traits derived from the raw keypoint data. This is necessary because different pipeline types will have different initial traits depending on which root types are tracked (e.g., primary + lateral, primary + main, only main, etc.).

An example of how these are used:

from sleap_roots import Series, DicotPipeline
plant = Series.load(r"tests\data\soy_6do\6PR6AA22JK.h5", primary_name="primary_multi_day", lateral_name="lateral__nodes")
pipeline = DicotPipeline()

# Plant-level traits
traits = pipeline.compute_plant_traits(plant)
assert traits.shape == (72, 115)

# Batch level traits
all_traits = pipeline.compute_batch_traits([plant])
assert all_traits.shape == (1, 1018)

Lin Wang and others added 30 commits July 20, 2023 15:31
…se graph and add comments. Delete duplicate `primary_depth`.
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #51 (e71e8d9) into main (1d30dbd) will decrease coverage by 4.51%.
The diff coverage is 86.15%.

@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
- Coverage   81.40%   76.90%   -4.51%     
==========================================
  Files          13       13              
  Lines         726      762      +36     
==========================================
- Hits          591      586       -5     
- Misses        135      176      +41     
Files Changed Coverage Δ
sleap_roots/convhull.py 76.11% <73.33%> (-6.74%) ⬇️
sleap_roots/points.py 78.57% <75.00%> (-18.49%) ⬇️
sleap_roots/lengths.py 76.92% <76.92%> (ø)
sleap_roots/networklength.py 81.48% <79.66%> (-11.06%) ⬇️
sleap_roots/tips.py 85.18% <81.81%> (-8.57%) ⬇️
sleap_roots/bases.py 88.46% <89.15%> (-3.13%) ⬇️
sleap_roots/angle.py 91.66% <89.74%> (-8.34%) ⬇️
sleap_roots/scanline.py 88.23% <92.30%> (-6.51%) ⬇️
sleap_roots/trait_pipelines.py 93.33% <93.33%> (ø)
sleap_roots/__init__.py 100.00% <100.00%> (ø)
... and 2 more

... and 1 file with indirect coverage changes

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

@talmo talmo linked an issue Aug 17, 2023 that may be closed by this pull request
pts_all_array: np.ndarray,
primary_length: float,
lateral_lengths: Union[float, np.ndarray],
network_length_lower: float,
fraction: float = 2 / 3,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete fraction as not used in this function

lateral_lengths: Lateral root lengths. Can be a single float (for one root)
or an array of floats (for multiple roots).
network_length_lower: The root length in the lower network.
fraction: The fraction of the network considered as 'lower'. Defaults to 2/3.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete the fraction

@eberrigan eberrigan merged commit d4015f4 into main Aug 17, 2023
5 checks passed
@eberrigan eberrigan deleted the talmo/pipeline_class branch August 17, 2023 16:52
eberrigan added a commit that referenced this pull request Mar 14, 2024
* Reorganizing the trait_map and modify ellipse, network functions.

* Update ellipse argument default setting.

* Reorganize ellipse and network functions by reducing arguments.

* Map convex hull traits

* Change primary to lateral when monocots is True

* Get tips x and y coordinates uses network map

* Change "stem" to "root"

* Fix tip y map

* Change root width back to take lateral_pts

* Changing the order of positional arguments to match others (primary is first)

* Fix plotting for using sleap-io API

* Make positional arguments consistent

* Refactor `get_base_xs` to use graph

* Map `scanline_intersection_counts` and use keyword arguments

* Refactor `get_base_ys`, `get_base_length`, and `base_ct_density` to use graph and add comments. Delete duplicate `primary_depth`.

* Clean up dependencies. Fix tip_ys.

* Refactor `get_root_lengths_max` for use with graph

* Refactor `get_base_tip_dist` to make base and tip pts or all points optional arguments

* Delete `primary_depth`

* Delete traitsgraph

* Delete traitsgraph dependencies

* Refactor base-related traits to use graph optionally

* Delete traits graph dependency

* Use `get_primary_pts` from series class

* Delete `get_primary_depth` tests

* Fix trait map for base traits

* Delete test for traitsgraph.py

* Standardize trait definition in trait map

* Change "graph" to "trait"

* Fix docstrings in `get_bases`

* Use `TraitDef` class

* Fix docstrings

* Add argument to class `TraitDef` whether to include in csv or if scalar

* Change `attr` to `attrs`

* Add `lengths.py` for length-related traits.

* Add `primary_max_length_pts` to trait definitions

* Add `pts_all_array` and `convex_hull` trait definitions

* Fix docstring

* Import base-related trait to `lengths.py`

* Make sure arrays of points are 2-dimensional

* Streamline point-related functions

* Vectorize `get_node_ind`

* Add trait definitions until `lateral_lengths`

* Delete unnecessary code

* Use node_ind for `get_root_angle` function.

* Modify base functions by assuming primary_pts as the primary_length_max.

* Modify argument pts as Optional in `get_base_tip_dist` function

* Modify argument pts as Optional in `get_grav_index` function

* Draft the trait_definitions using the defined TraitDef class.

* Uppercase the `get_root_angle` function arg description.

* Add test_lengths module for lengths-related functions.

* Remove lengths-related functions from test_bases.

* Set pts as Optional argument for `get_grav_index` function.

* Change the module name for importing lengths-related functions.

* Remove importing the points functions, only keep `get_all_pts_array`.

* Test ellipse-related functions.

* Redo the function `get_node_ind`.

* Test function `get_node_ind`.

* Angle function reset node_ind to array if only one value.

* Angle function return nan if all Nan node, return value if single array.

* Test angle functions.

* Add network_width_depth_ratio in trait_definitions.

* Reorganize arguments of `get_network_distribution_ratio` function.

* Add `network_length` trait before calculating `network_solidity`.

* Update `primary_root_length` function with calculated lengths.

* Update `get_network_solidity` function with calculated network_length.

* Test network-related functions.

* Test points function (`get_all_pts_array`).

* Update and test scanline functions using calculated scanline counts.

* Refactor `get_root_pair_widths_projections` to take in `primary_max_length_pts`

* Cleanup trait map

* Fix tests for base-related traits

* Add test for `get_max_lengths_pts`

* Refactored `get_base_ct_density` to take `primary_length_max` and `lateral_base_pts` as arguments

* Fixed multi-line strings

* Refactor base-related traits

* Refactor base-related traits and tests

* Test root-length-related traits

* Test tip-related traits

* Refactor convex-hull-related traits

* Test convex-hull functions

* Lint

* Lint

* Lint

* Lint

* Lint

* Fix kwargs involving `get_tips` in trait map

* Fix input for pipeline tests

* Refactor network related functions

* Test pipeline

* Refactor scanline function

* Start refactoring pipeline into classes

* Finish refactoring trait pipelines into classes

* Runtime fixes

* More refactoring to minimize redundant code across pipeline types

* Rename module and fix tests

* Add missing renamed modules

* Fix summary tests

* Fix Series to load video directly to bypass path resolution issues

* Lint

* Lint

---------

Co-authored-by: Lin Wang <[email protected]>
Co-authored-by: Elizabeth Berrigan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential speedups in pipeline runtime
3 participants