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 inference script #51

Merged
merged 42 commits into from
Jun 3, 2024
Merged

Refactor inference script #51

merged 42 commits into from
Jun 3, 2024

Conversation

aaprasad
Copy link
Contributor

@aaprasad aaprasad commented May 27, 2024

Its been a while since I updated the inference script so here I've updated it to work with our API

Summary by CodeRabbit

  • New Features

    • Added new settings for persistent tracking and anchor configuration in the inference pipeline.
  • Bug Fixes

    • Corrected parameter order issues in dataset initialization to ensure proper functionality.
    • Fixed handling of track IDs during inference to ensure accurate tracking.
  • Refactor

    • Renamed and updated function signatures in the inference module for improved clarity and functionality.
    • Modified configuration handling to support additional parameters and improve flexibility.
  • Tests

    • Updated test configurations and added new test functions to cover recent changes and ensure robustness.

…`Instance` classes into separate modules within `io`
…`Instance` classes into separate modules within `io`
Copy link
Contributor

coderabbitai bot commented May 27, 2024

Walkthrough

The recent updates in the biogtr project involve significant changes to dataset handling, inference configurations, and tracking logic. Key modifications include parameter reordering in dataset constructors, new configuration settings for tracking, and enhanced methods for handling file paths and video processing. Additionally, the inference function has been renamed and updated to improve tracking accuracy and result handling. These changes collectively enhance the flexibility and functionality of the dataset processing and tracking workflow.

Changes

Files/Modules Change Summary
biogtr/datasets/base_dataset.py Updated BaseDataset constructor to use label_files and vid_files instead of files.
biogtr/datasets/cell_tracking_dataset.py Swapped the order of gt_images and raw_images parameters in the constructor.
biogtr/datasets/microscopy_dataset.py Changed the order of tracks and videos parameters in the constructor.
biogtr/datasets/sleap_dataset.py Split slp_files and video_files into separate arguments in the constructor.
biogtr/inference/configs/inference.yaml Added persistent_tracking and anchor settings in the configuration file.
biogtr/inference/track.py Renamed inference to track, added trainer parameter, and updated logic for tracking and result saving.
biogtr/inference/tracker.py Adjusted track initialization and ID handling during inference.
biogtr/io/config.py Added params_cfg parameter, new methods for loading config from YAML, and retrieving file paths.
biogtr/io/frame.py Updated to_slp method to handle track_lookup and video loading.
biogtr/io/instance.py Modified to_slp method to convert self.pose to a numpy array and updated exception messages.
biogtr/models/gtr_runner.py Removed setting persistent_tracking to True in predict_step function.
tests/configs/inference.yaml Introduced new configurations for tracking and dataset settings for testing.
tests/fixtures/configs.py Added inference_config pytest fixture to retrieve inference params config file path.
tests/test_datasets.py Updated DummyDataset initialization to include label_files and vid_files.
tests/test_inference.py Added new test function test_track and imports for Config and run.
tests/test_training.py Added directory creation with tmp_path and updated hparams dictionary in test configuration.

Poem

In fields of code where changes bloom,
The datasets dance, no longer fume.
With labels clear and videos bright,
They track the paths both day and night.
Configs set and tests in place,
The code now runs a smoother race.
A rabbit's joy, in bytes and lines,
For data's flow in perfect signs.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@talmo talmo changed the base branch from main to aadi/expose-modules May 28, 2024 21:42
biogtr/datasets/base_dataset.py Outdated Show resolved Hide resolved
@talmo talmo marked this pull request as ready for review May 29, 2024 21:22
Base automatically changed from aadi/expose-modules to main June 3, 2024 22:51
@aaprasad aaprasad merged commit 63506c8 into main Jun 3, 2024
3 checks passed
@aaprasad aaprasad deleted the aadi/refactor-inference branch June 3, 2024 23:50
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Outside diff range and nitpick comments (9)
tests/test_training.py (2)

Line range hint 9-9: Remove unused import.

- from omegaconf import DictConfig

Line range hint 44-44: Remove unused variable.

- feats = 128
biogtr/datasets/microscopy_dataset.py (1)

Line range hint 86-86: Replace the lambda expression with a function definition for better readability and maintainability.

- parser = lambda x: data_utils.parse_synthetic(x, source=source)
+ def parser(x):
+     return data_utils.parse_synthetic(x, source=source)
tests/test_inference.py (1)

Line range hint 168-168: Remove the unused local variable N to clean up the code.

- N = N_t * T
Tools
Ruff

4-4: pytest imported but unused


7-7: omegaconf.DictConfig imported but unused

biogtr/models/gtr_runner.py (1)

Line range hint 78-78: Correct the type hints to use proper string formatting.

- ref_instances: list["biogtr.io.Instance"],
- query_instances: list["biogtr.io.Instance"] = None,
+ ref_instances: list[Instance],
+ query_instances: list[Instance] = None,

Also applies to: 79-79, 94-94, 112-112, 130-130, 148-148, 149-149, 166-166

tests/test_datasets.py (1)

Line range hint 11-11: Remove the unused import to clean up the code.

- from biogtr.models.model_utils.get_device
biogtr/datasets/sleap_dataset.py (1)

Line range hint 9-9: Remove unused import to clean up the code.

- import warnings
biogtr/io/frame.py (1)

Line range hint 161-183: Ensure consistent handling of default parameters in to_slp.

The method to_slp modifies the default parameter track_lookup which can lead to unexpected behavior across function calls.

- def to_slp(self, track_lookup: dict[int, sio.Track] = None):
+ def to_slp(self, track_lookup: dict[int, sio.Track] = None):
+     if track_lookup is None:
+         track_lookup = {}
biogtr/inference/tracker.py (1)

Line range hint 158-475: Refactor sliding_inference to improve clarity and reduce complexity.

The method sliding_inference is complex and contains nested logic that could be simplified. Consider breaking down the method into smaller, more manageable functions.

+ def handle_instance_features(self, frame, model):
+     # Logic to handle instance features
+     pass
+
+ def update_tracks(self, frame, batch_idx):
+     # Logic to update tracks
+     pass
+
- # Existing complex logic
+ self.handle_instance_features(frame, model)
+ self.update_tracks(frame, batch_idx)
Tools
Ruff

174-174: Undefined name curr_track


174-174: Local variable curr_track is assigned to but never used

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1ad8a47 and f725848.

Files selected for processing (16)
  • biogtr/datasets/base_dataset.py (3 hunks)
  • biogtr/datasets/cell_tracking_dataset.py (1 hunks)
  • biogtr/datasets/microscopy_dataset.py (1 hunks)
  • biogtr/datasets/sleap_dataset.py (1 hunks)
  • biogtr/inference/configs/inference.yaml (1 hunks)
  • biogtr/inference/track.py (3 hunks)
  • biogtr/inference/tracker.py (4 hunks)
  • biogtr/io/config.py (7 hunks)
  • biogtr/io/frame.py (2 hunks)
  • biogtr/io/instance.py (2 hunks)
  • biogtr/models/gtr_runner.py (1 hunks)
  • tests/configs/inference.yaml (1 hunks)
  • tests/fixtures/configs.py (1 hunks)
  • tests/test_datasets.py (1 hunks)
  • tests/test_inference.py (2 hunks)
  • tests/test_training.py (1 hunks)
Additional context used
Ruff
tests/test_training.py

9-9: omegaconf.DictConfig imported but unused


44-44: Local variable feats is assigned to but never used

biogtr/inference/track.py

17-17: Undefined name biogtr


118-118: f-string without any placeholders

biogtr/datasets/microscopy_dataset.py

86-86: Do not assign a lambda expression, use a def

tests/test_inference.py

4-4: pytest imported but unused


7-7: omegaconf.DictConfig imported but unused


168-168: Local variable N is assigned to but never used

biogtr/models/gtr_runner.py

11-11: biogtr.io.frame.Frame imported but unused


12-12: biogtr.io.instance.Instance imported but unused


78-78: Undefined name biogtr


79-79: Undefined name biogtr


94-94: Undefined name biogtr


112-112: Undefined name biogtr


130-130: Undefined name biogtr


148-148: Undefined name biogtr


149-149: Undefined name biogtr


166-166: Undefined name biogtr

tests/test_datasets.py

11-11: biogtr.models.model_utils.get_device imported but unused

biogtr/datasets/sleap_dataset.py

9-9: warnings imported but unused

biogtr/io/config.py

56-56: Undefined name params_cfg


82-82: Undefined name GlobalTrackingTransformer


94-94: Undefined name GTRRunner


110-110: Undefined name GTRRunner


167-167: Undefined name SleapDataset


167-167: Undefined name MicroscopyDataset


167-167: Undefined name CellTrackingDataset


231-231: Undefined name SleapDataset


231-231: Undefined name MicroscopyDataset


231-231: Undefined name CellTrackingDataset


302-302: Undefined name biogtr

biogtr/io/frame.py

57-57: Undefined name Instance


58-58: Undefined name AssociationMatrix


299-299: Undefined name Instance


308-308: Undefined name Instance


338-338: Undefined name AssociationMatrix


357-357: Undefined name AssociationMatrix

biogtr/inference/tracker.py

120-120: f-string without any placeholders


174-174: Undefined name curr_track


174-174: Local variable curr_track is assigned to but never used

biogtr/io/instance.py

93-93: Undefined name Frame


520-520: Undefined name Frame


529-529: Undefined name Frame

Additional comments not posted (12)
tests/configs/inference.yaml (1)

7-7: Ensure persistent_tracking is set as intended.

Verification successful

The persistent_tracking setting is correctly utilized in the tracking logic within the codebase.

  • biogtr/models/gtr_runner.py: Multiple references to persistent_tracking, including its initialization and usage.
  • biogtr/inference/tracker.py: Initialization and conditional logic based on persistent_tracking.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the `persistent_tracking` setting is correctly utilized in the tracking logic.

# Test: Search for usage of `persistent_tracking` in the codebase. Expect: Proper handling in tracking logic.
rg --type python $'persistent_tracking'

Length of output: 69



Script:

#!/bin/bash
# Description: Verify that the `persistent_tracking` setting is correctly utilized in the tracking logic.

# Test: Search for usage of `persistent_tracking` in Python files. Expect: Proper handling in tracking logic.
rg 'persistent_tracking' --glob '*.py'

Length of output: 1096

biogtr/inference/configs/inference.yaml (1)

7-7: Ensure persistent_tracking is set as intended.

Verification successful

The persistent_tracking setting is correctly utilized in the tracking logic.

  • biogtr/inference/tracker.py: The persistent_tracking attribute is defined, documented, and used in the tracking logic.
  • biogtr/models/gtr_runner.py: The persistent_tracking attribute is defined, documented, and used in the runner logic.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the `persistent_tracking` setting is correctly utilized in the tracking logic.

# Test: Search for usage of `persistent_tracking` in the codebase. Expect: Proper handling in tracking logic.
rg --type python $'persistent_tracking'

Length of output: 69



Script:

#!/bin/bash
# Description: Verify that the `persistent_tracking` setting is correctly utilized in the tracking logic.

# Test: Search for usage of `persistent_tracking` in the codebase. Expect: Proper handling in tracking logic.
rg --type py 'persistent_tracking'

Length of output: 1092

tests/fixtures/configs.py (1)

25-29: Added fixture for inference configuration.

This will help in managing the inference configuration paths more effectively.

tests/test_training.py (1)

126-141: Ensure directories are created before use.

This change improves the test setup by ensuring necessary directories are created.

biogtr/inference/track.py (1)

54-88: Refactored tracking logic.

The refactoring enhances clarity and modularity by separating concerns and improving readability.

biogtr/datasets/base_dataset.py (1)

Line range hint 16-49: Updated constructor parameters.

The changes to the constructor parameters enhance flexibility and clarity, allowing for more granular control over dataset initialization.

biogtr/datasets/microscopy_dataset.py (1)

54-55: Parameter order change in the constructor is consistent with the PR objectives and improves clarity.

biogtr/datasets/cell_tracking_dataset.py (1)

58-59: Parameter order change in the constructor is consistent with the PR objectives and improves clarity.

biogtr/datasets/sleap_dataset.py (2)

68-69: The addition of video_files to the constructor is well-integrated and aligns with the PR objectives.


68-69: The usage of video_files in get_instances method is correctly implemented and supports the functionality described in the PR.
[APROVED]

biogtr/io/instance.py (2)

230-230: Convert self.pose values to a numpy array to ensure compatibility with sio.PredictedInstance.from_numpy.

Ensure this change is covered by unit tests to verify the correct data type is passed.


241-241: Enhanced error message provides more context for debugging.

Consider adding logging for such errors in a production environment to aid in monitoring and troubleshooting.

Comment on lines +144 to +164
def get_data_paths(self, data_cfg: dict) -> tuple[list[str], list[str]]:
"""Get file paths from directory.

Args:
data_cfg: Config for the dataset containing "dir" key.

Returns:
lists of labels file paths and video file paths respectively
"""
dir_cfg = data_cfg.pop("dir", None)

if dir_cfg:
labels_suff = dir_cfg.labels_suffix
vid_suff = dir_cfg.vid_suffix

label_files = glob.glob(f"{dir_cfg.path}/*.{labels_suff}")
vid_files = glob.glob(f"{dir_cfg.path}/*.{vid_suff}")
return label_files, vid_files

return None, None

Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor get_data_paths to improve error handling and return type consistency.

The method get_data_paths currently returns None, None if dir_cfg is not found. It would be more consistent to always return two lists, even if they are empty.

- return None, None
+ return [], []
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def get_data_paths(self, data_cfg: dict) -> tuple[list[str], list[str]]:
"""Get file paths from directory.
Args:
data_cfg: Config for the dataset containing "dir" key.
Returns:
lists of labels file paths and video file paths respectively
"""
dir_cfg = data_cfg.pop("dir", None)
if dir_cfg:
labels_suff = dir_cfg.labels_suffix
vid_suff = dir_cfg.vid_suffix
label_files = glob.glob(f"{dir_cfg.path}/*.{labels_suff}")
vid_files = glob.glob(f"{dir_cfg.path}/*.{vid_suff}")
return label_files, vid_files
return None, None
def get_data_paths(self, data_cfg: dict) -> tuple[list[str], list[str]]:
"""Get file paths from directory.
Args:
data_cfg: Config for the dataset containing "dir" key.
Returns:
lists of labels file paths and video file paths respectively
"""
dir_cfg = data_cfg.pop("dir", None)
if dir_cfg:
labels_suff = dir_cfg.labels_suffix
vid_suff = dir_cfg.vid_suffix
label_files = glob.glob(f"{dir_cfg.path}/*.{labels_suff}")
vid_files = glob.glob(f"{dir_cfg.path}/*.{vid_suff}")
return label_files, vid_files
return [], []

Comment on lines +47 to +57
@classmethod
def from_yaml(cls, base_cfg_path: str, params_cfg_path: str = None) -> None:
"""Load config directly from yaml.

Args:
base_cfg_path: path to base config file.
params_cfg_path: path to override params.
"""
base_cfg = OmegaConf.load(base_cfg_path)
params_cfg = OmegaConf.load(params_cfg_path) if params_cfg else None
return cls(base_cfg, params_cfg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure proper handling of None for optional parameters in from_yaml.

The method from_yaml does not handle the case where params_cfg_path is None before attempting to load it, which could lead to an error.

- params_cfg = OmegaConf.load(params_cfg_path) if params_cfg else None
+ if params_cfg_path:
+     params_cfg = OmegaConf.load(params_cfg_path)
+ else:
+     params_cfg = None
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
@classmethod
def from_yaml(cls, base_cfg_path: str, params_cfg_path: str = None) -> None:
"""Load config directly from yaml.
Args:
base_cfg_path: path to base config file.
params_cfg_path: path to override params.
"""
base_cfg = OmegaConf.load(base_cfg_path)
params_cfg = OmegaConf.load(params_cfg_path) if params_cfg else None
return cls(base_cfg, params_cfg)
@classmethod
def from_yaml(cls, base_cfg_path: str, params_cfg_path: str = None) -> None:
"""Load config directly from yaml.
Args:
base_cfg_path: path to base config file.
params_cfg_path: path to override params.
"""
base_cfg = OmegaConf.load(base_cfg_path)
if params_cfg_path:
params_cfg = OmegaConf.load(params_cfg_path)
else:
params_cfg = None
return cls(base_cfg, params_cfg)
Tools
Ruff

56-56: Undefined name params_cfg

Comment on lines +190 to +222
label_files, vid_files = self.get_data_paths(dataset_params)
# todo: handle this better
if "slp_files" in dataset_params:
if label_files is not None:
dataset_params.slp_files = label_files
if vid_files is not None:
dataset_params.video_files = vid_files
return SleapDataset(**dataset_params)

elif "tracks" in dataset_params or "source" in dataset_params:
if label_files is not None:
dataset_params.tracks = label_files
if vid_files is not None:
dataset_params.video_files = vid_files
return MicroscopyDataset(**dataset_params)

elif "raw_images" in dataset_params:
if label_files is not None:
dataset_params.gt_images = label_files
if vid_files is not None:
dataset_params.raw_images = vid_files
return CellTrackingDataset(**dataset_params)

# todo: handle this better
if "slp_files" in dataset_params:
return SleapDataset(**dataset_params)

elif "tracks" in dataset_params or "source" in dataset_params:
return MicroscopyDataset(**dataset_params)

elif "raw_images" in dataset_params:
return CellTrackingDataset(**dataset_params)

Copy link
Contributor

Choose a reason for hiding this comment

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

Simplify the get_dataset method by reducing redundancy.

The method get_dataset contains repetitive blocks of code for handling different dataset types. This can be simplified by using a mapping from dataset type to class.

+ dataset_mapping = {
+     'slp_files': SleapDataset,
+     'tracks': MicroscopyDataset,
+     'source': MicroscopyDataset,
+     'raw_images': CellTrackingDataset
+ }
+ for key, dataset_class in dataset_mapping.items():
+     if key in dataset_params:
+         if label_files is not None:
+             dataset_params[key.replace('_files', '')] = label_files
+         if vid_files is not None:
+             dataset_params['video_files'] = vid_files
+         return dataset_class(**dataset_params)
+ raise ValueError("Could not resolve dataset type from Config!")
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
label_files, vid_files = self.get_data_paths(dataset_params)
# todo: handle this better
if "slp_files" in dataset_params:
if label_files is not None:
dataset_params.slp_files = label_files
if vid_files is not None:
dataset_params.video_files = vid_files
return SleapDataset(**dataset_params)
elif "tracks" in dataset_params or "source" in dataset_params:
if label_files is not None:
dataset_params.tracks = label_files
if vid_files is not None:
dataset_params.video_files = vid_files
return MicroscopyDataset(**dataset_params)
elif "raw_images" in dataset_params:
if label_files is not None:
dataset_params.gt_images = label_files
if vid_files is not None:
dataset_params.raw_images = vid_files
return CellTrackingDataset(**dataset_params)
# todo: handle this better
if "slp_files" in dataset_params:
return SleapDataset(**dataset_params)
elif "tracks" in dataset_params or "source" in dataset_params:
return MicroscopyDataset(**dataset_params)
elif "raw_images" in dataset_params:
return CellTrackingDataset(**dataset_params)
label_files, vid_files = self.get_data_paths(dataset_params)
dataset_mapping = {
'slp_files': SleapDataset,
'tracks': MicroscopyDataset,
'source': MicroscopyDataset,
'raw_images': CellTrackingDataset
}
for key, dataset_class in dataset_mapping.items():
if key in dataset_params:
if label_files is not None:
dataset_params[key.replace('_files', '')] = label_files
if vid_files is not None:
dataset_params['video_files'] = vid_files
return dataset_class(**dataset_params)
raise ValueError("Could not resolve dataset type from Config!")

Comment on lines +16 to +37
def __init__(self, cfg: DictConfig, params_cfg: DictConfig = None):
"""Initialize the class with config from hydra/omega conf.

First uses `base_param` file then overwrites with specific `params_config`.

Args:
cfg: The `DictConfig` containing all the hyperparameters needed for
training/evaluation.
params_cfg: The `DictConfig` containing subset of hyperparameters to override.
training/evaluation
"""
base_cfg = cfg
print(f"Base Config: {cfg}")

if "params_config" in cfg:
# merge configs
params_config = OmegaConf.load(cfg.params_config)
pprint(f"Overwriting base config with {params_config}")
self.cfg = OmegaConf.merge(base_cfg, params_config)
params_cfg = OmegaConf.load(cfg.params_config)

if params_cfg:
pprint(f"Overwriting base config with {params_cfg}")
self.cfg = OmegaConf.merge(base_cfg, params_cfg) # merge configs
else:
# just use base config
self.cfg = base_cfg
self.cfg = cfg
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding error handling for file operations in __init__.

While loading configurations from files, it's crucial to handle potential I/O errors. Consider wrapping the file operations in a try-except block to catch and handle exceptions like FileNotFoundError or PermissionError.

+ try:
+     params_cfg = OmegaConf.load(cfg.params_config)
+ except Exception as e:
+     print(f"Error loading params config: {e}")
+     return None
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def __init__(self, cfg: DictConfig, params_cfg: DictConfig = None):
"""Initialize the class with config from hydra/omega conf.
First uses `base_param` file then overwrites with specific `params_config`.
Args:
cfg: The `DictConfig` containing all the hyperparameters needed for
training/evaluation.
params_cfg: The `DictConfig` containing subset of hyperparameters to override.
training/evaluation
"""
base_cfg = cfg
print(f"Base Config: {cfg}")
if "params_config" in cfg:
# merge configs
params_config = OmegaConf.load(cfg.params_config)
pprint(f"Overwriting base config with {params_config}")
self.cfg = OmegaConf.merge(base_cfg, params_config)
params_cfg = OmegaConf.load(cfg.params_config)
if params_cfg:
pprint(f"Overwriting base config with {params_cfg}")
self.cfg = OmegaConf.merge(base_cfg, params_cfg) # merge configs
else:
# just use base config
self.cfg = base_cfg
self.cfg = cfg
def __init__(self, cfg: DictConfig, params_cfg: DictConfig = None):
"""Initialize the class with config from hydra/omega conf.
First uses `base_param` file then overwrites with specific `params_config`.
Args:
cfg: The `DictConfig` containing all the hyperparameters needed for
training/evaluation.
params_cfg: The `DictConfig` containing subset of hyperparameters to override.
training/evaluation
"""
base_cfg = cfg
print(f"Base Config: {cfg}")
if "params_config" in cfg:
try:
params_cfg = OmegaConf.load(cfg.params_config)
except Exception as e:
print(f"Error loading params config: {e}")
return None
if params_cfg:
pprint(f"Overwriting base config with {params_cfg}")
self.cfg = OmegaConf.merge(base_cfg, params_cfg) # merge configs
else:
self.cfg = cfg

@@ -3,88 +3,13 @@
import torch
import pytest
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unused imports to clean up the code.

- import pytest
- from omegaconf import DictConfig

Also applies to: 7-7

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
import pytest
Tools
Ruff

4-4: pytest imported but unused

print(f"Pod Index: {index}")

checkpoints = pd.read_csv(cfg.checkpoints)
checkpoint = checkpoints.iloc[index]
else:
checkpoint = pred_cfg.get_ckpt_path()
checkpoint = pred_cfg.cfg.ckpt_path

model = GTRRunner.load_from_checkpoint(checkpoint)
tracker_cfg = pred_cfg.get_tracker_cfg()
print("Updating tracker hparams")
model.tracker_cfg = tracker_cfg
print(f"Using the following params for tracker:")
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unnecessary f-string.

- print(f"Using the following params for tracker:")
+ print("Using the following params for tracker:")
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
print(f"Using the following params for tracker:")
print("Using the following params for tracker:")
Tools
Ruff

118-118: f-string without any placeholders

@@ -11,6 +11,7 @@
import pandas as pd
import pytorch_lightning as pl
import torch
import sleap_io as sio


def export_trajectories(frames_pred: list["biogtr.io.Frame"], save_path: str = None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix undefined name error.

- from biogtr.io import Config
+ from biogtr.io import Config, Frame
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def export_trajectories(frames_pred: list["biogtr.io.Frame"], save_path: str = None):
from biogtr.io import Config, Frame
def export_trajectories(frames_pred: list["biogtr.io.Frame"], save_path: str = None):
Tools
Ruff

17-17: Undefined name biogtr

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.

2 participants