Top-down Centered-instance Pipeline #16
Conversation
… docstring to make_centered_bboxes, fixed find_global_peaks_rough; added crop_bboxes
…leupsamplingblock, encoder, decoder; added unet
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files selected for processing (6)
- sleap_nn/data/augmentation.py (6 hunks)
- sleap_nn/data/instance_centroids.py (1 hunks)
- sleap_nn/data/instance_cropping.py (2 hunks)
- sleap_nn/data/pipelines.py (1 hunks)
- tests/data/test_instance_cropping.py (1 hunks)
- tests/data/test_pipelines.py (1 hunks)
Files skipped from review due to trivial changes (1)
- sleap_nn/data/instance_cropping.py
Additional comments (Suppressed): 9
sleap_nn/data/instance_centroids.py (1)
- 87-93: The changes in this hunk are minimal and mostly cosmetic, changing the variable name
exampletoex. The logic remains the same and there doesn't seem to be any issues with it. However, ensure that the change of variable name does not affect other parts of the code whereexamplemight have been used.tests/data/test_instance_cropping.py (2)
29-29: The
InstanceCropperclass now takes a tuple for the crop size instead of separate width and height parameters. Ensure that all calls to this class throughout the codebase have been updated to match the new signature.35-35: The key for bounding box data in the sample dictionary has changed from
"bbox"to"instance_bbox". Make sure that any code that accesses this data is updated accordingly.sleap_nn/data/augmentation.py (6)
100-105: The attribute
rotationis unchanged and does not require any action.122-132: The attributes
random_crop_hwandrandom_crop_phave replaced the oldcrop_hwandcrop_p. Ensure that all references to these attributes in the codebase have been updated accordingly.150-170: The parameters of the constructor function have been updated. The changes include renaming
crop_hwandcrop_ptorandom_crop_hwandrandom_crop_p, respectively, and changing default values for some parameters. Make sure that all calls to this function throughout the codebase have been updated to match the new signature.188-192: The instance variables
random_crop_hwandrandom_crop_phave replaced the oldcrop_hwandcrop_p. Ensure that all references to these variables in the codebase have been updated accordingly.259-276: The condition for applying random crop has been updated to check for
random_crop_pinstead ofcrop_p, andrandom_crop_hwinstead ofcrop_hw. This change is consistent with the updates made to the class attributes and constructor parameters.285-312: The
__iter__method has been significantly updated to handle two different cases: when"instance_image"and"instance"keys are present in the example, and when"image"and"instances"keys are present. This change seems to improve the flexibility of the method by allowing it to handle different types of input data. However, ensure that the logic of the method still works as expected with these changes.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (2)
- sleap_nn/data/pipelines.py (1 hunks)
- tests/data/test_pipelines.py (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/data/test_pipelines.py
Additional comments (Suppressed): 1
sleap_nn/data/pipelines.py (1)
- 48-51: Ensure that all calls to this class throughout the codebase have been updated to match the new signature.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (1)
- sleap_nn/data/pipelines.py (1 hunks)
Additional comments (Suppressed): 1
sleap_nn/data/pipelines.py (1)
- 47-49: Ensure that all calls to this class throughout the codebase have been updated to match the new signature.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (1)
- sleap_nn/data/pipelines.py (1 hunks)
Additional comments (Suppressed): 5
sleap_nn/data/pipelines.py (5)
1-6:
7-16: Ensure that all the imported modules are used in the code and remove any unnecessary imports to keep the code clean and efficient.
18-31: The
SleapDatasetclass is well implemented. It takes anIterDataPipeas input and provides a dictionary with relevant outputs when iterated over.32-47: This block of code checks if the shape of
instance_imagehas 4 dimensions, and if so, it removes one dimension usingsqueeze(dim=0). This could potentially lead to unexpected results if the first dimension ofinstance_imagehas more than one element. Consider verifying this logic and its implications on the rest of the pipeline.50-60: The
TopdownConfmapsPipelineclass is initialized with adata_configwhich is aDictConfig. This configuration is used later in the pipeline creation methods.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Files selected for processing (11)
- sleap_nn/architectures/common.py (2 hunks)
- sleap_nn/data/augmentation.py (6 hunks)
- sleap_nn/data/confidence_maps.py (3 hunks)
- sleap_nn/data/general.py (1 hunks)
- sleap_nn/data/instance_centroids.py (2 hunks)
- sleap_nn/data/instance_cropping.py (2 hunks)
- sleap_nn/data/normalization.py (2 hunks)
- sleap_nn/data/pipelines.py (1 hunks)
- sleap_nn/data/providers.py (2 hunks)
- tests/data/test_instance_cropping.py (1 hunks)
- tests/data/test_pipelines.py (1 hunks)
Files skipped from review due to trivial changes (1)
- sleap_nn/data/providers.py
Additional comments (Suppressed): 25
sleap_nn/data/general.py (1)
- 1-38: The
KeyFilterclass is well implemented. It provides a way to filter keys from the data pipeline, which can be useful for focusing on specific parts of the data during processing. The use of type hints and clear comments enhances readability and maintainability.sleap_nn/data/normalization.py (2)
1-6: The import of
Dictfromtypingis added but it's not used in this hunk. Ensure that it's used in the code outside this hunk, otherwise, it's an unnecessary import.18-28: Type hints and return type annotations have been added to the
__init__and__iter__methods. This improves code readability and provides better documentation for the expected types of inputs and outputs.sleap_nn/architectures/common.py (2)
1-6: The import of
Listfromtypingis added but not used in the provided hunks. Ensure that it's used in the parts of the code not shown here, otherwise, it should be removed to avoid unnecessary imports.136-142: Type hinting has been added for the function
get_children_layers. This improves code readability and helps with static type checking.sleap_nn/data/instance_centroids.py (2)
1-5: The import of
Dictfrom thetypingmodule is new in this hunk. Ensure that it's used in the code that follows this hunk.79-93: The changes made to the
InstanceCentroidFinderclass are mostly cosmetic, with the exception of adding type hints and return type annotations to the__init__and__iter__methods. These changes improve the readability of the code and provide better documentation for the expected types of inputs and outputs.- def __init__( - self, - source_dp: IterDataPipe, - anchor_ind: Optional[int] = None, - ): + def __init__( + self, + source_dp: IterDataPipe, + anchor_ind: Optional[int] = None, + ) -> None: - def __iter__(self): + def __iter__(self) -> Dict[str, torch.Tensor]:sleap_nn/data/instance_cropping.py (4)
1-5: The import of
Tuplefrom thetypingmodule is a new addition. Ensure that it's used appropriately in the code.64-67: The constructor of
InstanceCroppernow takes a tuplecrop_hwinstead of separatecrop_widthandcrop_height. This change simplifies the function signature and enhances readability. However, make sure all calls to this constructor throughout the codebase have been updated to match the new signature.76-79: The
make_centered_bboxesfunction now takes height and width from thecrop_hwtuple. This is consistent with the changes made to the constructor.95-101: The keys in the
instance_exampledictionary have been updated. The key"bbox"has been changed to"instance_bbox", and the key"instance_image"now includes a call tosqueeze(0). These changes improve clarity and consistency in the code. However, ensure that all references to these keys in the rest of the codebase have been updated accordingly.sleap_nn/data/confidence_maps.py (2)
1-5: The import statement
from typing import Dict, Optionalhas been updated to includeDict. This change is likely due to the addition of type hints in the code. Ensure that all uses ofDictare correctly implemented.10-16: The function signature for
make_confmaps()has been updated with a return type annotation-> torch.Tensor. This improves code readability and provides better documentation for the expected output type of the function.sleap_nn/data/augmentation.py (5)
100-105: The attribute
crop_hwhas been removed and replaced withrandom_crop_hw. Ensure that all references tocrop_hwin the codebase have been updated torandom_crop_hw.122-132: The default values for
erase_scale,erase_ratio, andrandom_crop_phave been changed. Make sure these changes do not affect the expected behavior of the data augmentation pipeline.150-170: The default probabilities for applying affine transformations, uniform noise, Gaussian noise, contrast adjustment, brightness adjustment, random erasing, and mixup have been set to 0. This means these augmentations will not be applied by default. If this is intended, it's fine. Otherwise, you might want to adjust these default values.
259-276: The check for
random_crop_hwbeing greater than 0 has been added. This is a good practice as it prevents unexpected behavior when the crop size is 0. However, ensure that this does not conflict with any use cases where a crop size of 0 might be intentionally used.285-312: The logic for handling examples with
"instance_image"and"instance"keys has been added. This improves the flexibility of the data augmentation pipeline by allowing it to handle different types of input data. However, make sure that this new logic is covered by the test suite to ensure it works as expected.tests/data/test_instance_cropping.py (2)
29-29: Ensure that all calls to
InstanceCropperthroughout the codebase have been updated to match the new signature, which now takes a tuple for the crop size.46-47: The shape assertions for "instance_image" and "instance_bbox" have changed from the old hunk. Ensure these changes are intentional and correctly reflect the expected shapes after modifications in the data processing pipeline.
sleap_nn/data/pipelines.py (5)
46-47: Ensure that
self.data_config.preprocessing.crop_hwis always defined and has valid values before passing it toInstanceCropper. If it's not defined or invalid, it could lead to unexpected behavior or errors.48-53: Check if
self.data_config.augmentation_config.random_cropis always defined and has valid values before accessing its attributes. If it's not defined or invalid, it could lead to unexpected behavior or errors.55-58: Ensure that
self.data_config.augmentation_config.augmentationsis always defined and has valid values before unpacking it as arguments toKorniaAugmenter. If it's not defined or invalid, it could lead to unexpected behavior or errors.60-64: Check if
self.data_config.preprocessing.conf_map_genis always defined and has valid values before accessing its attributes. If it's not defined or invalid, it could lead to unexpected behavior or errors.65-66: Ensure that
self.data_config.general.keep_keysis always defined and has valid values before passing it toKeyFilter. If it's not defined or invalid, it could lead to unexpected behavior or errors.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (8)
- sleap_nn/data/augmentation.py (7 hunks)
- sleap_nn/data/confidence_maps.py (3 hunks)
- sleap_nn/data/general.py (1 hunks)
- sleap_nn/data/instance_centroids.py (2 hunks)
- sleap_nn/data/instance_cropping.py (2 hunks)
- sleap_nn/data/normalization.py (2 hunks)
- sleap_nn/data/providers.py (2 hunks)
- tests/data/test_instance_cropping.py (1 hunks)
Additional comments (Suppressed): 23
sleap_nn/data/normalization.py (2)
1-6: The addition of typing imports is a good practice for better code readability and static type checking. This change aligns with the goals of the PR.
18-28: The addition of return type hints in the function signatures improves the clarity of the code and provides better static type checking. This change also aligns with the goals of the PR.
sleap_nn/data/instance_centroids.py (2)
1-5: The import of
DictandIteratorfrom thetypingmodule is a good addition for better type hinting and code readability.79-93: The changes in this hunk are mostly cosmetic, with no significant alterations to the logic or functionality. The introduction of type hints improves code readability and provides better static type checking.
sleap_nn/data/confidence_maps.py (3)
1-5: The import statement
from typing import Dict, Iterator, Optionalhas been updated to includeDictandIterator. This is a good practice as it improves code readability and provides better static type checking.10-16: The function signature for
make_confmaps()has been updated to include a return type hint. This is a good practice as it improves code readability and provides better static type checking.70-84: The method signature for the
__init__()and__iter__()methods of theConfidenceMapGeneratorclass have been updated to include return type hints. This is a good practice as it improves code readability and provides better static type checking.sleap_nn/data/instance_cropping.py (2)
1-5: The import of
Dict,Iterator, andTuplefrom thetypingmodule is a good addition for better type hinting and code readability.64-67: The change to use a single attribute
crop_hw(a tuple) instead of separatecrop_widthandcrop_heightattributes simplifies the function signature and makes it easier to manage. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature.sleap_nn/data/providers.py (4)
1-6: The import of
DictandIteratorfrom thetypingmodule is new. Ensure that these types are used appropriately in the rest of the code.21-23: Type hint for the
labelsparameter has been added to the constructor. This improves readability and provides better static type checking.26-29: Type hint for the
filenameparameter and return type hint have been added to thefrom_filenamemethod. This enhances code readability and provides better static type checking.31-34: Return type hint has been added to the
__iter__method. This improves code readability and provides better static type checking.tests/data/test_instance_cropping.py (3)
29-29: The
InstanceCropperclass now takes a tuple for the crop size instead of separate width and height parameters. Ensure that all calls to this class throughout the codebase have been updated to match the new signature.46-46: The shape of the "instance_image" has changed from (1, 1, 100, 100) to (1, 100, 100). This change should be verified across the codebase to ensure it doesn't break any functionality that relies on the old shape.
47-47: The key for bounding box data in the sample dictionary has changed from "bbox" to "instance_bbox". Make sure to update all references to this key in the codebase.
sleap_nn/data/augmentation.py (7)
1-5: The import statement
from typing import Iteratorhas been added. Ensure that this new import is used in the code.100-105: The attribute
crop_hwandcrop_phave been removed from the class. Make sure that these attributes are not being referenced anywhere else in the codebase.122-132: The default values for
erase_scaleanderase_ratiohave been changed. Verify if these changes align with the intended functionality of the code.150-170: The parameters
affine_p,uniform_noise_p,gaussian_noise_p,contrast_p,brightness_p,erase_p, andmixup_phave their default values changed to 0.0. This means these augmentations will be disabled by default. If this is intentional, no action is needed. Otherwise, consider reverting these changes.188-192: The attribute
crop_hwandcrop_phave been replaced withrandom_crop_hwandrandom_crop_p. Make sure that these new attributes are correctly used in the rest of the code.259-276: The cropping operation has been updated to use the new attributes
random_crop_hwandrandom_crop_p. The logic seems correct as it checks if both height and width are greater than 0 before applying the crop.282-312: The
__iter__method has been significantly modified. It now handles two cases: when both "instance_image" and "instance" keys are present in the example, and when both "image" and "instances" keys are present. The augmentation is applied accordingly. This change appears to improve the flexibility of the method to handle different types of input data.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (2)
- sleap_nn/data/general.py (1 hunks)
- sleap_nn/data/instance_cropping.py (2 hunks)
Additional comments (Suppressed): 4
sleap_nn/data/general.py (1)
- 1-39: The
KeyFilterclass is well implemented. It provides a way to filter keys from the data dictionary based on the providedkeep_keyslist. The logic in the__iter__method correctly handles both cases wherekeep_keysis provided or not. The use of type hints and docstrings enhances readability and understanding of the code.sleap_nn/data/instance_cropping.py (3)
1-5: The import statement
from typing import Dict, Iterator, Optional, Tuplehas been added. Ensure that these types are used appropriately throughout the code.58-67: The
InstanceCropperclass now takes a tuple for crop dimensions instead of separate height and width parameters. This is a good change as it simplifies the function signature and makes the code more maintainable.64-67: Ensure that all calls to this class throughout the codebase have been updated to match the new signature.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (1)
- sleap_nn/data/instance_cropping.py (2 hunks)
Additional comments (Suppressed): 1
sleap_nn/data/instance_cropping.py (1)
- 1-5: The import statement has been updated to include additional types
Dict,Iterator, andTuplefor better type hinting. This is a good practice as it improves code readability and allows for static type checking.
Summary by CodeRabbit
SleapDatasetandTopdownConfmapsPipelineclasses for high-level pipeline configurations in data processing.crop_widthandcrop_heightattributes into a singlecrop_hwattribute in theInstanceCropperclass for better clarity and maintainability.KorniaAugmenterclass to userandom_crop_hwandrandom_crop_pparameters, improving flexibility in data augmentation.test_sleap_datasetandtest_topdownconfmapspipelineto validate the functionality of the newly introduced classes.