Set up eval pipeline #67
Conversation
WalkthroughThe recent updates to the DREEM framework enhance the user experience by streamlining command-line interactions, modernizing type annotations, and improving code clarity. New documentation offers clear guidance on utilizing features, while robust logging capabilities facilitate better debugging and monitoring. Additionally, the introduction of structured logging and evaluation configurations enriches overall functionality and maintainability throughout the project. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Training Script
participant Inference Script
User->>CLI: Run command (e.g., dreem-train)
CLI->>Training Script: Execute training process
Training Script->>CLI: Return training results
User->>CLI: Run command (e.g., dreem-eval)
CLI->>Inference Script: Execute evaluation process
Inference Script->>CLI: Return evaluation results
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Outside diff range, codebase verification and nitpick comments (6)
docs/cli.md (4)
13-13: Specify language for the code block.To improve readability and comply with Markdownlint rules, specify the language for the code block.
- ``` + ```plaintextTools
Markdownlint
13-13: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
86-86: Possible missing comma.There might be a missing comma after "param".
- param we recommend setting up a visualize config + param, we recommend setting up a visualize configTools
LanguageTool
[uncategorized] ~86-~86: Possible missing comma found.
Context: ...pecifying each individual+annotate.*param we recommend setting up a visualize con...(AI_HYDRA_LEO_MISSING_COMMA)
35-35: Specify language for the code block.To improve readability and comply with Markdownlint rules, specify the language for the code block.
- ``` + ```plaintextTools
Markdownlint
35-35: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
56-56: Specify language for the code block.To improve readability and comply with Markdownlint rules, specify the language for the code block.
- ``` + ```plaintextTools
Markdownlint
56-56: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
dreem/training/losses.py (1)
49-51: Reintroduce error handling in theforwardmethod.The removal of the
try-exceptblock around thetorch.catoperation could lead to unhandledRuntimeErrorexceptions, potentially impacting the robustness of the method. It is recommended to reintroduce the error handling to manage these exceptions effectively.
dreem/training/losses.py:try: target_inst_id = torch.cat( [frame.get_gt_track_ids().to(asso_preds[-1].device) for frame in frames] ) except RuntimeError as e: # Handle the exception appropriately passAnalysis chain
Verify the removal of error handling.
The removal of the
try-exceptblock for handlingRuntimeErrorduring tensor concatenation could impact the method's robustness. Ensure that this change does not introduce potential issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing error handling in the `forward` method. # Test: Search for the `forward` method usage. Expect: No issues introduced due to the removal of error handling. rg --type python -A 5 $'forward'Length of output: 62
Script:
#!/bin/bash # Description: Verify the impact of removing error handling in the `forward` method. # Search for the `forward` method implementation in the codebase. ast-grep --lang python --pattern $'def forward($_$) { $$$ }' # Search for the `forward` method usage in the codebase. rg 'forward\(' --type py -A 5Length of output: 5597
Script:
#!/bin/bash # Description: Review the `forward` method in `dreem/training/losses.py` and its surrounding context. # Display the `forward` method and its surrounding lines in `dreem/training/losses.py`. rg 'def forward' -A 20 -B 5 dreem/training/losses.pyLength of output: 1000
dreem/models/gtr_runner.py (1)
276-310: Theon_test_epoch_endmethod is not invoked anywhere in the codebase.
- The method
on_test_epoch_enddefined indreem/models/gtr_runner.pyis not called or used anywhere in the codebase. Ensure that this method is correctly invoked where necessary.Analysis chain
LGTM! New method
on_test_epoch_endenhances functionality.The new method
on_test_epoch_endcorrectly handles saving test results to an HDF5 file usingpandasandh5py.However, ensure that the method is correctly invoked in the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the invocation of `on_test_epoch_end` method. # Test: Search for the method invocation. Expect: Correct invocation of the new method. rg --type python -A 5 $'on_test_epoch_end'Length of output: 72
Script:
#!/bin/bash # Description: Verify the invocation of `on_test_epoch_end` method. # Test: Search for the method invocation. Expect: Correct invocation of the new method. rg -t py -A 5 'on_test_epoch_end'Length of output: 402
Script:
#!/bin/bash # Description: Verify the invocation of `on_test_epoch_end` method across the entire codebase. # Test: Search for the method invocation. Expect: Correct invocation of the new method. rg 'on_test_epoch_end'Length of output: 82
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (42)
- README.md (6 hunks)
- docs/cli.md (1 hunks)
- docs/index.md (1 hunks)
- docs/usage.md (5 hunks)
- dreem/init.py (2 hunks)
- dreem/datasets/base_dataset.py (5 hunks)
- dreem/datasets/cell_tracking_dataset.py (3 hunks)
- dreem/datasets/data_utils.py (6 hunks)
- dreem/datasets/eval_dataset.py (2 hunks)
- dreem/datasets/microscopy_dataset.py (3 hunks)
- dreem/datasets/sleap_dataset.py (4 hunks)
- dreem/datasets/tracking_dataset.py (2 hunks)
- dreem/inference/boxes.py (6 hunks)
- dreem/inference/configs/eval.yaml (1 hunks)
- dreem/inference/eval.py (1 hunks)
- dreem/inference/metrics.py (4 hunks)
- dreem/inference/post_processing.py (3 hunks)
- dreem/inference/track.py (4 hunks)
- dreem/inference/track_queue.py (7 hunks)
- dreem/inference/tracker.py (12 hunks)
- dreem/io/association_matrix.py (5 hunks)
- dreem/io/config.py (16 hunks)
- dreem/io/frame.py (14 hunks)
- dreem/io/instance.py (10 hunks)
- dreem/io/track.py (2 hunks)
- dreem/io/visualize.py (8 hunks)
- dreem/models/embedding.py (4 hunks)
- dreem/models/global_tracking_transformer.py (2 hunks)
- dreem/models/gtr_runner.py (6 hunks)
- dreem/models/model_utils.py (4 hunks)
- dreem/models/transformer.py (6 hunks)
- dreem/models/visual_encoder.py (3 hunks)
- dreem/training/init.py (1 hunks)
- dreem/training/losses.py (4 hunks)
- dreem/training/train.py (5 hunks)
- environment.yml (1 hunks)
- environment_cpu.yml (1 hunks)
- environment_osx-arm64.yml (1 hunks)
- logging.yaml (1 hunks)
- mkdocs.yml (1 hunks)
- pyproject.toml (2 hunks)
- tests/test_datasets.py (1 hunks)
Files skipped from review due to trivial changes (7)
- docs/index.md
- dreem/datasets/eval_dataset.py
- dreem/datasets/tracking_dataset.py
- dreem/inference/post_processing.py
- dreem/training/init.py
- mkdocs.yml
- tests/test_datasets.py
Additional context used
yamllint
dreem/inference/configs/eval.yaml
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 24-24: no new line character at the end of file
(new-line-at-end-of-file)
[error] 24-24: trailing spaces
(trailing-spaces)
logging.yaml
[error] 36-36: trailing spaces
(trailing-spaces)
Ruff
dreem/__init__.py
3-3:
logging.configimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
34-34: Local variable
loggeris assigned to but never usedRemove assignment to unused variable
logger(F841)
dreem/io/track.py
80-80: Undefined name
Instance(F821)
80-80: Undefined name
Instance(F821)
dreem/inference/eval.py
7-7:
pathlib.Pathimported but unusedRemove unused import:
pathlib.Path(F401)
12-12:
pytorch_lightningimported but unusedRemove unused import:
pytorch_lightning(F401)
13-13:
torchimported but unusedRemove unused import:
torch(F401)
28-28: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
45-45: f-string without any placeholders
Remove extraneous
fprefix(F541)
48-48: f-string without any placeholders
Remove extraneous
fprefix(F541)
62-62: Local variable
metricsis assigned to but never usedRemove assignment to unused variable
metrics(F841)
dreem/inference/track.py
21-21: Undefined name
dreem(F821)
113-113: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/models/model_utils.py
8-8: Undefined name
dreem(F821)
33-33: Undefined name
dreem(F821)
dreem/training/losses.py
36-36: Undefined name
Frame(F821)
dreem/models/gtr_runner.py
86-86: Undefined name
dreem(F821)
87-87: Undefined name
dreem(F821)
dreem/io/config.py
115-115: Undefined name
GTRRunner(F821)
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
257-257: Undefined name
SleapDataset(F821)
257-257: Undefined name
MicroscopyDataset(F821)
257-257: Undefined name
CellTrackingDataset(F821)
dreem/inference/tracker.py
142-142: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/models/transformer.py
145-145: Undefined name
dreem(F821)
dreem/io/frame.py
63-63: Undefined name
AssociationMatrix(F821)
229-229: Multiple statements on one line (colon)
(E701)
363-363: Undefined name
Instance(F821)
372-372: Undefined name
Instance(F821)
dreem/io/instance.py
247-247: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
268-268: Undefined name
pred_track_id(F821)
LanguageTool
docs/cli.md
[uncategorized] ~86-~86: Possible missing comma found.
Context: ...pecifying each individual+annotate.*param we recommend setting up a visualize con...(AI_HYDRA_LEO_MISSING_COMMA)
docs/usage.md
[grammar] ~139-~139: It seems like one article is redundant in this context.
Context: .../dreem_configs` directory that contains a only a small selection of parameters that I'd like t...(A_RB_A_JJ_NN)
[uncategorized] ~151-~151: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...E_CONFIG_STEM] section.param=value ``` e.g If now I want to override a couple para...(E_G)
[grammar] ~152-~152: Using ‘couple’ without ‘of’ is considered to be informal.
Context: ...ue ``` e.g If now I want to override a couple parameters again, say change the number of attenti...(PLENTY_OF_NOUNS)
[uncategorized] ~247-~247: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...[CONFIG_STEM] section.param=[VALUE] ``` e.g if I want to set the window size of the...(E_G)
[typographical] ~253-~253: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...pecified in the config it will save to./[OUTDIR]/[VID_NAME].dreem_inference.slp, otherwise it will just save to `./results/[VID_NA...(THUS_SENTENCE)
README.md
[uncategorized] ~156-~156: Possible missing comma found.
Context: ...nside my/home/aaprasad/dreem_configsdirectory I can call ```bash dreem-train --config...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~175-~175: It seems like one article is redundant in this context.
Context: .../dreem_configs` directory that contains a only a small selection of parameters that I'd like t...(A_RB_A_JJ_NN)
[uncategorized] ~182-~182: Possible missing comma found.
Context: ...g a specific param via the command line directly you can use thesection.param=keysyn...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~187-~187: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...E_CONFIG_STEM] section.param=value ``` e.g If now I want to override a couple para...(E_G)
[grammar] ~188-~188: Using ‘couple’ without ‘of’ is considered to be informal.
Context: ...ue ``` e.g If now I want to override a couple parameters again, say change the number of attenti...(PLENTY_OF_NOUNS)
[uncategorized] ~280-~280: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...[CONFIG_STEM] section.param=[VALUE] ``` e.g if I want to set the window size of the...(E_G)
Markdownlint
docs/cli.md
13-13: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
35-35: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
56-56: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (133)
environment_osx-arm64.yml (1)
9-9: Verify compatibility with Python 3.11.The Python version has been updated from 3.9 to 3.11. Ensure that all dependencies are compatible with Python 3.11 to avoid potential issues.
environment_cpu.yml (1)
10-10: Verify compatibility with Python 3.11.The Python version has been updated from 3.9 to 3.11. Ensure that all dependencies are compatible with Python 3.11 to avoid potential issues.
Verification successful
Python 3.11 Compatibility Verified
The project includes Python 3.11 in its
pyproject.tomlfile and multiple environment files, indicating that it supports Python 3.11. No explicit compatibility issues were found in the repository.
pyproject.toml: Python 3.11 is listed in the classifiers.environment_osx-arm64.yml,environment_cpu.yml,environment.yml: Python 3.11 is specified.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of dependencies with Python 3.11. # Test: Create a new environment and install dependencies. Expect: No errors during installation. conda create -n test_env python=3.11 conda activate test_env conda install -c conda-forge opencv=4.8.0 pytorch cpuonly torchvision lightning imageio av ffmpeg albumentations matplotlib pip pip install matplotlib sleap-io --editable=.[dev] imageio[ffmpeg] hydra-core motmetrics seaborn wandb timmLength of output: 597
Script:
#!/bin/bash # Search for mentions of Python version compatibility in the repository # Search for Python version mentions in README.md and other documentation files rg "Python 3\.[0-9]+" README.md docs/ -A 3 # Search for Python version mentions in setup.py or other configuration files rg "python_requires" setup.py # Search for any issues or notes regarding Python 3.11 compatibility in the codebase rg "3\.11" . -A 3Length of output: 15130
environment.yml (1)
10-10: Verify compatibility with Python 3.11.The Python version has been updated from 3.9 to 3.11. Ensure that all dependencies are compatible with Python 3.11 to avoid potential issues.
dreem/inference/configs/eval.yaml (4)
1-1: LGTM!The
ckpt_pathis correctly specified.
2-7: LGTM!The
trackerparameters are correctly specified.
9-15: LGTM!The
datasetparameters are correctly specified.
17-20: LGTM!The
dataloaderparameters are correctly specified.logging.yaml (5)
1-1: LGTM!The
versionis correctly specified.
2-4: LGTM!The
formatterssection is correctly specified.
5-7: LGTM!The
handlerssection is correctly specified.
8-31: LGTM!The
loggerssection is correctly specified.
32-34: LGTM!The
rootlogger is correctly specified.dreem/__init__.py (1)
23-23: LGTM!The docstring is correctly specified.
pyproject.toml (2)
21-22: Approved: Python version classifiers update.The addition of Python 3.10 and 3.11 classifiers is consistent with modern Python practices and ensures broader compatibility.
58-61: Approved: Project scripts update.The new script entries (
dreem-train,dreem-eval,dreem-track,dreem-visualize) are correctly defined and point to the appropriate modules and functions.However, ensure that all script definitions are correctly implemented in the respective modules.
Verification successful
Let's create new shell scripts to search for the function definitions directly within the specified files.
Approved: Project scripts update.
The new script entries (
dreem-train,dreem-eval,dreem-track,dreem-visualize) are correctly defined and point to the appropriate modules and functions.
dreem-train->dreem.training.train:rundreem-eval->dreem.inference.eval:rundreem-track->dreem.inference.track:rundreem-visualize->dreem.io.visualize:mainScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the script definitions in the respective modules. # Test: Search for the function definitions. Expect: All functions should be defined in the respective modules. rg --type python -A 5 'def run' | grep -E 'dreem/training/train.py|dreem/inference/eval.py|dreem/inference/track.py' rg --type python -A 5 'def main' | grep -E 'dreem/io/visualize.py'Length of output: 247
Script:
#!/bin/bash # Description: Verify the existence of the script definitions in the respective modules. # Test: Search for the function definitions. Expect: All functions should be defined in the respective modules. # Check for 'run' function in the specified files. rg 'def run' dreem/training/train.py dreem/inference/eval.py dreem/inference/track.py # Check for 'main' function in the specified file. rg 'def main' dreem/io/visualize.pyLength of output: 353
dreem/io/track.py (1)
80-80: Approved: Type annotations update.The use of the new syntax for type unions in Python 3.10 enhances readability and aligns with modern Python practices.
However, ensure that the
Instanceclass is correctly defined and imported.Verification successful
Approved: Type annotations update.
The use of the new syntax for type unions in Python 3.10 enhances readability and aligns with modern Python practices. The
Instanceclass is correctly defined indreem/io/instance.py.
dreem/io/instance.py:class InstanceScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the `Instance` class. # Test: Search for the class definition. Expect: The `Instance` class should be defined in the codebase. rg --type python -A 5 'class Instance'Length of output: 71
Script:
#!/bin/bash # Description: Verify the existence of the `Instance` class. # Test: Search for the class definition. Expect: The `Instance` class should be defined in the codebase. rg --type py -A 5 'class Instance'Length of output: 409
Tools
Ruff
80-80: Undefined name
Instance(F821)
80-80: Undefined name
Instance(F821)
dreem/training/train.py (4)
18-19: LGTM!The addition of the logging module is appropriate for enhancing logging capabilities.
20-20: LGTM!Initializing the logger with a specific name helps in distinguishing logs from different modules.
Line range hint
78-90:
LGTM!The changes improve logging practices and avoid potential conflicts.
49-50: LGTM! But verify the function usage in the codebase.The code changes are approved. The logging improvements are appropriate.
However, ensure that all function calls to
runmatch the new logging practices.Verification successful
Let's correct the regex pattern and re-run the search to verify the function usage of
run.
Let's update the shell script to use the
--globflag to include Python files and re-run the search.
The search results show multiple instances of the
runfunction being used in different files. We need to ensure that the logging practices introduced indreem/training/train.pyare consistent across these usages.
LGTM! But verify the function usage in the codebase.
The code changes are approved. The logging improvements are appropriate for
dreem/training/train.py. The other instances of therunfunction do not require the same logging practices.
tests/test_inference.pytests/test_training.pydreem/inference/track.pydreem/inference/eval.pyThese files do not need changes to match the new logging practices.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `run` match the new logging practices. # Test: Search for the function usage. Expect: Only occurances of the new logging practices. rg --type python -A 5 $'run('Length of output: 89
Script:
#!/bin/bash # Description: Verify all function calls to `run` match the new logging practices. # Corrected Test: Search for the function usage. Expect: Only occurrences of the new logging practices. rg --type python -A 5 'run\('Length of output: 62
Script:
#!/bin/bash # Description: Verify all function calls to `run` match the new logging practices. # Corrected Test: Search for the function usage. Expect: Only occurrences of the new logging practices. rg --glob '*.py' -A 5 'run\('Length of output: 1399
Script:
#!/bin/bash # Description: Review the context around the `run` function calls to verify logging practices. # Extract context around `run` function calls in identified files. rg --glob '*.py' -A 10 -B 10 'run\('Length of output: 6812
dreem/inference/track.py (7)
15-16: LGTM!The addition of the logging module is appropriate for enhancing logging capabilities.
17-17: LGTM!Initializing the logger with a specific name helps in distinguishing logs from different modules.
21-21: LGTM!The changes enhance type safety and improve the function's flexibility.
Tools
Ruff
21-21: Undefined name
dreem(F821)
73-83: LGTM!The changes improve efficiency and maintainability.
101-101: LGTM!The logging improvements are appropriate.
110-110: LGTM!The logging improvements are appropriate.
116-130: LGTM!The logging improvements are appropriate.
dreem/models/visual_encoder.py (3)
3-3: LGTM!The addition of the
Anytype is appropriate for enhancing type annotations.
27-27: LGTM!The changes enhance type clarity and improve type checking and code readability.
60-60: LGTM!The changes enhance type clarity and improve type checking and code readability.
dreem/models/global_tracking_transformer.py (2)
15-15: LGTM! Modernized type annotation forencoder_cfg.The type annotation change to
dict | Noneis correct and aligns with modern Python conventions.
26-26: LGTM! Modernized type annotation forembedding_meta.The type annotation change to
dict | Noneis correct and aligns with modern Python conventions.docs/cli.md (1)
88-98: LGTM! Example visualize config is clear and comprehensive.The example configuration for the
dreem-visualizecommand is well-documented and easy to understand.dreem/datasets/base_dataset.py (7)
22-22: LGTM! Modernized type annotation foraugmentations.The type annotation change to
dict | Noneis correct and aligns with modern Python conventions.
23-23: LGTM! Modernized type annotation forn_chunks.The type annotation change to
int | floatis correct and aligns with modern Python conventions.
24-24: LGTM! Modernized type annotation forseed.The type annotation change to
int | Noneis correct and aligns with modern Python conventions.
25-25: LGTM! Modernized type annotation forgt_list.The type annotation change to
str | Noneis correct and aligns with modern Python conventions.
124-124: LGTM! Simplified return type annotation forno_batching_fn.The return type annotation change to
list[Frame]is correct and aligns with modern Python conventions.
135-135: LGTM! Simplified return type annotation for__getitem__.The return type annotation change to
list[Frame]is correct and aligns with modern Python conventions.
162-162: LGTM! Simplified parameter type annotations forget_instances.The parameter type annotations change to
list[int]is correct and aligns with modern Python conventions.dreem/datasets/microscopy_dataset.py (1)
25-27: Type hint improvements approved.The updated type hints for
augmentations,n_chunks, andseedparameters enhance type safety and readability.dreem/models/model_utils.py (3)
8-8: Type hint improvement approved.The updated type hint for the
instancesparameter enhances readability and maintains compatibility with existing functionality.Tools
Ruff
8-8: Undefined name
dreem(F821)
33-33: Type hint improvement approved.The updated type hint for the
query_instancesparameter enhances clarity and readability.Tools
Ruff
33-33: Undefined name
dreem(F821)
170-170: Type hint improvement approved.The updated type hint for the
configparameter standardizes the handling of optional parameters across the module.dreem/inference/boxes.py (5)
40-40: Type hint improvement approved.The updated return type hint for the
clonemethod enhances clarity and correctness.
48-48: Type hint improvement approved.The updated return type hint for the
tomethod enhances clarity and correctness.
69-69: Type hint improvement approved.The updated parameter type hint for the
clipmethod indicates a preference for a list structure over a tuple for box size specifications.
105-105: Type hint improvement approved.The updated parameter type hint for the
__getitem__method modernizes the type hinting syntax.
184-184: Type hint improvement approved.The updated parameter type hint for the
catmethod aligns with the new type hinting conventions.dreem/datasets/cell_tracking_dataset.py (2)
26-29: LGTM! Type annotation updates improve readability.The changes to type annotations using the modern union type syntax enhance the readability and maintainability of the code.
118-118: LGTM! Type annotation updates improve readability.The return type annotation update using the modern type hinting style enhances the readability and maintainability of the code.
dreem/training/losses.py (2)
36-36: LGTM! Type annotation updates improve readability.The changes to type annotations using built-in Python types enhance the readability and maintainability of the code.
Tools
Ruff
36-36: Undefined name
Frame(F821)
105-105: LGTM! Type annotation updates improve readability.The return type annotation update using the modern type hinting style enhances the readability and maintainability of the code.
dreem/inference/metrics.py (4)
7-10: LGTM! Logging enhancements improve traceability.The addition of logging capabilities using the
loggingmodule improves the traceability and debugging of the code.
45-46: LGTM! Logging enhancements improve traceability.The addition of a debug log message when no instances are detected improves the traceability and debugging of the code.
238-240: LGTM! Type annotation updates improve readability.The changes to type annotations using the modern union type syntax enhance the readability and maintainability of the code.
197-197: Verify the removal of error handling.The removal of the
try-exceptblock for assigninggt_idssimplifies the code but could introduce potential issues ifgt_idsare not available. Ensure that this change does not introduce potential issues.dreem/inference/track_queue.py (4)
161-161: LGTM! Type hint update improves clarity.The change to allow
Nonefor thetrack_idparameter enhances type safety and clarity.
217-219: LGTM! Logging statements improve debugging.The added logging statements provide useful information about new tracks, enhancing debugging capabilities.
228-230: LGTM! Type hint updates improve clarity.The changes to allow
Nonefor thetrack_idsanddeviceparameters enhance type safety and clarity.
Line range hint
293-307:
LGTM! Logging statements improve debugging.The added logging statements provide useful information about track gaps and terminations, enhancing debugging capabilities.
dreem/io/visualize.py (4)
Line range hint
66-75:
LGTM! Type hint updates and logging improve clarity and debugging.The changes to allow
list | strforcolor_paletteandfloatforalphaenhance type safety and clarity. The added logging statements and improved documentation provide better debugging capability and usability.
98-102: LGTM! Logging statements improve debugging.The added logging statements provide useful information about the color palette, enhancing debugging capabilities.
267-267: LGTM! Exception handling with logging improves robustness.The change to use logging for exception handling instead of print statements provides better context and traceability for errors.
328-330: LGTM! Logging statements improve debugging.The added logging statements provide useful information about the video saving process, enhancing debugging capabilities.
dreem/io/association_matrix.py (5)
24-24: LGTM! Type hint updates improve clarity.The change to use
np.ndarray | torch.Tensorfor thematrixattribute enhances type safety and clarity.
170-170: LGTM! Type hint update improves clarity.The change to allow
Nonefor therow_groupingparameter enhances type safety and clarity.
226-226: LGTM! Type hint update improves clarity.The change to use
int | Instance | list[int | Instance]for theindsparameter enhances type safety and clarity.
256-256: LGTM! Type hint update improves clarity.The change to use
Instance | int | np.typing.ArrayLikefor theinstanceparameter enhances type safety and clarity.
247-251: LGTM! Exception handling with logging improves robustness.The change to use logging for exception handling instead of print statements provides better context and traceability for errors.
dreem/models/gtr_runner.py (4)
5-7: LGTM! New imports are appropriate.The new imports for
logging,pandas, andh5pyare necessary for the added functionality.
16-16: LGTM! Logger instance initialization is appropriate.The logger instance is correctly initialized for structured logging.
87-87: LGTM! Updated type annotations improve clarity and type safety.The updated type annotations in the
forwardmethod enhance code clarity and type safety.Tools
Ruff
87-87: Undefined name
dreem(F821)
39-46: LGTM! Constructor changes improve clarity and functionality.The updated type annotations and the new parameter
test_save_pathenhance code clarity and add functionality for saving test results.However, ensure that all instantiations of
GTRRunnermatch the new signature.dreem/datasets/sleap_dataset.py (4)
9-9: LGTM! New import forloggingis appropriate.The new import for
loggingis necessary for the added logging functionality.
14-14: LGTM! Logger instance initialization is appropriate.The logger instance is correctly initialized for structured logging.
26-33: LGTM! Constructor changes improve clarity.The updated type annotations in the constructor enhance code clarity and modernize the type hints.
Line range hint
128-172:
LGTM! Updated type annotations and logging improve clarity and error handling.The updated type annotations in the
get_instancesmethod enhance code clarity, and the logging replaces print statements for structured error handling.dreem/models/embedding.py (5)
5-5: LGTM! New import forloggingis appropriate.The new import for
loggingis necessary for the added logging functionality.
8-8: LGTM! Logger instance initialization is appropriate.The logger instance is correctly initialized for structured logging.
35-41: LGTM! Constructor changes improve clarity.The updated type annotations in the constructor enhance code clarity and modernize the type hints.
287-290: LGTM! Updated logging improves error handling.The logging in the
_learned_pos_embeddingmethod replaces print statements for structured error handling.
340-340: LGTM! Updated type annotations improve clarity.The updated type annotations in the
_compute_weightsmethod enhance code clarity and modernize the type hints.dreem/datasets/data_utils.py (3)
55-55: LGTM! Modern type hinting syntax approved.The type hint update for the
sizeparameter to use the pipe operator (|) is correct and aligns with modern Python practices.
110-110: LGTM! Modern type hinting syntax approved.The type hint update for the
bbox_sizeparameter to use the pipe operator (|) is correct and aligns with modern Python practices.
496-496: LGTM! Modern type hinting syntax approved.The type hint update for the
instancesparameter to use lowercase forlistanddictis correct and aligns with modern Python practices.dreem/io/config.py (7)
19-19: LGTM! Modern type hinting syntax approved.The type hint update for the
params_cfgparameter to use the pipe operator (|) is correct and aligns with modern Python practices.
52-52: LGTM! Modern type hinting syntax approved.The type hint update for the
params_cfg_pathparameter to use the pipe operator (|) is correct and aligns with modern Python practices.
115-115: LGTM! New parameter addition approved.The addition of the
ckpt_pathparameter to theget_gtr_runnermethod is correctly integrated and enhances the method's flexibility.Tools
Ruff
115-115: Undefined name
GTRRunner(F821)
196-200: LGTM! Modern type hinting syntax and new parameters approved.The type hint updates for the
label_filesandvid_filesparameters to use the pipe operator (|) and the addition of the new parameters are correctly integrated and enhance the method's flexibility.Tools
Ruff
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
257-257: LGTM! Modern type hinting syntax approved.The type hint updates for the
datasetparameter to use the pipe operator (|) are correct and align with modern Python practices.Tools
Ruff
257-257: Undefined name
SleapDataset(F821)
257-257: Undefined name
MicroscopyDataset(F821)
257-257: Undefined name
CellTrackingDataset(F821)
386-386: LGTM! Logging updates approved.The update to use logging for exceptions in the
get_checkpointingmethod is correctly integrated and enhances the method's maintainability.
407-407: LGTM! Modern type hinting syntax approved.The type hint updates for the
callbacksandloggerparameters to use the pipe operator (|) are correct and align with modern Python practices.docs/usage.md (4)
115-115: LGTM! Updated training command approved.The command for training has been correctly updated to
dreem-train, which simplifies the command usage.
121-121: LGTM! Updated training command example approved.The example command for training has been correctly updated to use
dreem-train, which simplifies the command usage.
136-136: LGTM! Updated training command with file-based override approved.The command for training with file-based override has been correctly updated to use
dreem-train, which simplifies the command usage.
231-231: LGTM! Updated inference command approved.The command for inference has been correctly updated to
dreem-track, which simplifies the command usage.README.md (8)
152-152: LGTM!The change simplifies the command for training.
158-158: LGTM!The change simplifies the command for training with a specific config.
172-172: LGTM!The change simplifies the command for overriding arguments.
178-178: LGTM!The change simplifies the command for overriding arguments with specific params.
185-185: LGTM!The change provides a clear example of how to use the CLI-based override.
191-191: LGTM!The change provides a clear example of how to override specific params via CLI.
264-264: LGTM!The change simplifies the command for running inference.
279-279: LGTM!The change provides a clear example of how to override params during inference via CLI.
dreem/inference/tracker.py (6)
5-5: LGTM!The addition of the logging module is appropriate for enhancing logging capabilities.
15-15: LGTM!The initialization of the logger is appropriate for consistent logging throughout the module.
27-29: LGTM!The type hinting enhancements improve code readability and type safety.
79-94: LGTM!The addition of the
__repr__method provides a detailed string representation of the tracker instance, facilitating easier debugging and logging.
169-186: LGTM!The logging integration enhances the ability to monitor the internal state of the tracker during execution.
Line range hint
259-472:
LGTM!The logging integration enhances the ability to monitor the internal state of the tracker during execution.
dreem/models/transformer.py (6)
42-42: LGTM!The type hinting enhancement improves code readability and type safety.
145-145: LGTM!The type hinting enhancement improves code readability and type safety.
Tools
Ruff
145-145: Undefined name
dreem(F821)
384-385: LGTM!The type hinting enhancement improves code readability and type safety.
446-446: LGTM!The type hinting enhancement improves code readability and type safety.
488-488: LGTM!The type hinting enhancement improves code readability and type safety.
508-509: LGTM!The type hinting enhancement improves code readability and type safety.
dreem/io/frame.py (9)
Line range hint
16-27:
Modernized type annotations.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices.
Line range hint
101-134:
Modernized type annotations and return type.The use of the pipe (
|) operator for type annotations and changing the return type toSelfimproves readability and aligns with modern Python practices.
Line range hint
136-165:
Modernized type annotations and return type.The use of the pipe (
|) operator for type annotations and changing the return type toSelfimproves readability and aligns with modern Python practices.
Line range hint
168-201:
Modernized type annotations and added parameter.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices. The addition of thevideoparameter enhances the method's functionality.
202-241: New method added for HDF5 conversion.The
to_h5method is a valuable addition that converts aFrameinstance to an HDF5 group. The method is well-documented and the implementation appears sound.Tools
Ruff
229-229: Multiple statements on one line (colon)
(E701)
Line range hint
304-312:
Modernized type annotations.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices.
332-340: New property added for video path retrieval.The
vid_nameproperty is a valuable addition that retrieves the video path. The implementation is sound and provides useful functionality.
Line range hint
363-371:
Modernized type annotations.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices.Tools
Ruff
363-363: Undefined name
Instance(F821)
Line range hint
457-475:
Modernized type annotations and improved logging.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices. The improved logging for exceptions enhances error tracking and is a better practice for production code.dreem/io/instance.py (7)
Line range hint
15-27:
Modernized type annotations.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices.
Line range hint
35-50:
Modernized type annotations.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices.
Line range hint
138-155:
Modernized type annotations and return type.The use of the pipe (
|) operator for type annotations and changing the return type toSelfimproves readability and aligns with modern Python practices.
Line range hint
161-179:
Modernized type annotations and return type.The use of the pipe (
|) operator for type annotations and changing the return type toSelfimproves readability and aligns with modern Python practices.
Line range hint
223-247:
Improved error handling with logging.Replacing
logger.exceptionenhances logging practices and provides better error tracking.Tools
Ruff
247-247: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
268-268: Undefined name
pred_track_id(F821)
Line range hint
524-531:
Modernized type annotations.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices.
Line range hint
537-555:
Modernized type annotations.The use of the pipe (
|) operator for type annotations improves readability and aligns with modern Python practices.
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Fix trailing spaces and add a new line character at the end of the file.
Trailing spaces and missing new line character at the end of the file should be fixed.
-
-
+
+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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + |
Tools
yamllint
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 24-24: no new line character at the end of file
(new-line-at-end-of-file)
[error] 24-24: trailing spaces
(trailing-spaces)
logging.yaml
Outdated
| level: [INFO] | ||
| handlers: [empty] | ||
|
|
||
|
|
There was a problem hiding this comment.
Fix trailing spaces.
Trailing spaces should be fixed.
-
+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. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 36-36: trailing spaces
(trailing-spaces)
dreem/__init__.py
Outdated
| @@ -1,5 +1,6 @@ | |||
| """Top-level package for dreem.""" | |||
|
|
|||
| import logging.config | |||
There was a problem hiding this comment.
Remove unused import.
The import statement for logging.config is unused and should be removed.
- import logging.configCommittable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import logging.config |
Tools
Ruff
3-3:
logging.configimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
dreem/__init__.py
Outdated
| def setup_logging(): | ||
| """Setup logging based on `logging.yaml`.""" | ||
| import logging | ||
| import yaml | ||
| import os | ||
|
|
||
| package_directory = os.path.dirname(os.path.abspath(__file__)) | ||
|
|
||
| with open(os.path.join(package_directory, "..", "logging.yaml"), "r") as stream: | ||
| logging_cfg = yaml.load(stream, Loader=yaml.FullLoader) | ||
|
|
||
| logging.config.dictConfig(logging_cfg) | ||
| logger = logging.getLogger("dreem") |
There was a problem hiding this comment.
Remove unused variable logger.
The local variable logger is assigned but never used and should be removed.
- logger = logging.getLogger("dreem")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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def setup_logging(): | |
| """Setup logging based on `logging.yaml`.""" | |
| import logging | |
| import yaml | |
| import os | |
| package_directory = os.path.dirname(os.path.abspath(__file__)) | |
| with open(os.path.join(package_directory, "..", "logging.yaml"), "r") as stream: | |
| logging_cfg = yaml.load(stream, Loader=yaml.FullLoader) | |
| logging.config.dictConfig(logging_cfg) | |
| logger = logging.getLogger("dreem") | |
| def setup_logging(): | |
| """Setup logging based on `logging.yaml`.""" | |
| import logging | |
| import yaml | |
| import os | |
| package_directory = os.path.dirname(os.path.abspath(__file__)) | |
| with open(os.path.join(package_directory, "..", "logging.yaml"), "r") as stream: | |
| logging_cfg = yaml.load(stream, Loader=yaml.FullLoader) | |
| logging.config.dictConfig(logging_cfg) |
Tools
Ruff
34-34: Local variable
loggeris assigned to but never usedRemove assignment to unused variable
logger(F841)
| model = GTRRunner.load_from_checkpoint(checkpoint) | ||
| model.tracker_cfg = eval_cfg.cfg.tracker | ||
| model.tracker = Tracker(**model.tracker_cfg) | ||
| logger.info(f"Using the following tracker:") |
There was a problem hiding this comment.
Remove extraneous f-string prefix.
The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Using the following tracker:")
+ logger.info("Using the following 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info(f"Using the following tracker:") | |
| logger.info("Using the following tracker:") |
Tools
Ruff
45-45: f-string without any placeholders
Remove extraneous
fprefix(F541)
| logger.info(f"Using the following tracker:") | ||
| print(model.tracker) | ||
| model.metrics["test"] = eval_cfg.cfg.runner.metrics.test | ||
| logger.info(f"Computing the following metrics:") |
There was a problem hiding this comment.
Remove extraneous f-string prefix.
The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Computing the following metrics:")
+ logger.info("Computing the following metrics:")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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info(f"Computing the following metrics:") | |
| logger.info("Computing the following metrics:") |
Tools
Ruff
48-48: f-string without any placeholders
Remove extraneous
fprefix(F541)
| label_files=[label_file], vid_files=[vid_file], mode="test" | ||
| ) | ||
| dataloader = eval_cfg.get_dataloader(dataset, mode="test") | ||
| metrics = trainer.test(model, dataloader) |
There was a problem hiding this comment.
Remove unused variable assignment.
The variable metrics is assigned but never used. Remove the assignment to clean up the code.
- metrics = trainer.test(model, dataloader)
+ trainer.test(model, dataloader)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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| metrics = trainer.test(model, dataloader) | |
| trainer.test(model, dataloader) |
Tools
Ruff
62-62: Local variable
metricsis assigned to but never usedRemove assignment to unused variable
metrics(F841)
| dataset = pred_cfg.get_dataset(mode="test") | ||
| dataloader = pred_cfg.get_dataloader(dataset, mode="test") | ||
| model.tracker = Tracker(**model.tracker_cfg) | ||
| logger.info(f"Using the following tracker:") |
There was a problem hiding this comment.
Remove extraneous f prefix.
The f-string does not contain any placeholders.
- logger.info(f"Using the following tracker:")
+ logger.info("Using the following 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info(f"Using the following tracker:") | |
| logger.info("Using the following tracker:") |
Tools
Ruff
113-113: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/inference/tracker.py
Outdated
| @@ -120,8 +139,7 @@ def track( | |||
| instances_pred = self.sliding_inference(model, frames) | |||
|
|
|||
| if not self.persistent_tracking: | |||
| if self.verbose: | |||
| warnings.warn(f"Clearing Queue after tracking") | |||
| logger.debug(f"Clearing Queue after tracking") | |||
There was a problem hiding this comment.
Remove extraneous f prefix.
The f-string is not using any placeholders, so the f prefix is unnecessary.
- logger.debug(f"Clearing Queue after tracking")
+ logger.debug("Clearing Queue after tracking")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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.debug(f"Clearing Queue after tracking") | |
| logger.debug("Clearing Queue after tracking") |
Tools
Ruff
142-142: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/io/instance.py
Outdated
| def to_h5(self, frame_group=h5py.Group, label=None, **kwargs: dict) -> h5py.Group: | ||
| """Convert instance to an h5 group". | ||
|
|
||
| By default we always save: | ||
| - the gt/pred track id | ||
| - bbox | ||
| - centroid | ||
| - pose | ||
| - instance/traj/points score | ||
| Larger arrays (crops/features/embeddings) can be saved by passing as kwargs | ||
|
|
||
| Args: | ||
| frame_group: the h5py group representing the frame the instance appears on | ||
| label: the name of the instance group that will be created | ||
| **kwargs: additional key:value pairs to be saved as datasets. | ||
| Returns: | ||
| The h5 group representing this instance. | ||
| """ | ||
| if label is None: | ||
| if pred_track_id != -1: | ||
| label = f"instance_{self.pred_track_id.item()}" | ||
| else: | ||
| label = f"instance_{self.gt_track_id.item()}" | ||
| instance_group = frame_group.create_group(label) | ||
| instance_group.attrs.create("gt_track_id", self.gt_track_id.item()) | ||
| instance_group.attrs.create("pred_track_id", self.pred_track_id.item()) | ||
| instance_group.attrs.create("track_score", self.track_score) | ||
| instance_group.attrs.create("instance_score", self.instance_score) | ||
|
|
||
| instance_group.create_dataset("bbox", data=self.bbox.cpu().numpy()) | ||
|
|
||
| pose_group = instance_group.create_group("pose") | ||
| pose_group.create_dataset("points", data=np.array(list(self.pose.values()))) | ||
| pose_group.attrs.create("nodes", list(self.pose.keys())) | ||
| pose_group.create_dataset("scores", data=self.point_scores) | ||
|
|
||
| for key, value in kwargs.items(): | ||
| if "emb" in key: | ||
| emb_group = instance_group.require_group("emb") | ||
| emb_group.create_dataset(key, data=value) | ||
| else: | ||
| instance_group.create_dataset(key, data=value) | ||
|
|
||
| return instance_group |
There was a problem hiding this comment.
New method added for HDF5 conversion.
The to_h5 method is a valuable addition that converts an Instance to an HDF5 group. The method is well-documented and the implementation appears sound.
Fix undefined variable pred_track_id.
The variable pred_track_id is undefined and should be replaced with self.pred_track_id.
- if pred_track_id != -1:
- label = f"instance_{pred_track_id.item()}"
+ if self.pred_track_id != -1:
+ label = f"instance_{self.pred_track_id.item()}"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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def to_h5(self, frame_group=h5py.Group, label=None, **kwargs: dict) -> h5py.Group: | |
| """Convert instance to an h5 group". | |
| By default we always save: | |
| - the gt/pred track id | |
| - bbox | |
| - centroid | |
| - pose | |
| - instance/traj/points score | |
| Larger arrays (crops/features/embeddings) can be saved by passing as kwargs | |
| Args: | |
| frame_group: the h5py group representing the frame the instance appears on | |
| label: the name of the instance group that will be created | |
| **kwargs: additional key:value pairs to be saved as datasets. | |
| Returns: | |
| The h5 group representing this instance. | |
| """ | |
| if label is None: | |
| if pred_track_id != -1: | |
| label = f"instance_{self.pred_track_id.item()}" | |
| else: | |
| label = f"instance_{self.gt_track_id.item()}" | |
| instance_group = frame_group.create_group(label) | |
| instance_group.attrs.create("gt_track_id", self.gt_track_id.item()) | |
| instance_group.attrs.create("pred_track_id", self.pred_track_id.item()) | |
| instance_group.attrs.create("track_score", self.track_score) | |
| instance_group.attrs.create("instance_score", self.instance_score) | |
| instance_group.create_dataset("bbox", data=self.bbox.cpu().numpy()) | |
| pose_group = instance_group.create_group("pose") | |
| pose_group.create_dataset("points", data=np.array(list(self.pose.values()))) | |
| pose_group.attrs.create("nodes", list(self.pose.keys())) | |
| pose_group.create_dataset("scores", data=self.point_scores) | |
| for key, value in kwargs.items(): | |
| if "emb" in key: | |
| emb_group = instance_group.require_group("emb") | |
| emb_group.create_dataset(key, data=value) | |
| else: | |
| instance_group.create_dataset(key, data=value) | |
| return instance_group | |
| def to_h5(self, frame_group=h5py.Group, label=None, **kwargs: dict) -> h5py.Group: | |
| """Convert instance to an h5 group". | |
| By default we always save: | |
| - the gt/pred track id | |
| - bbox | |
| - centroid | |
| - pose | |
| - instance/traj/points score | |
| Larger arrays (crops/features/embeddings) can be saved by passing as kwargs | |
| Args: | |
| frame_group: the h5py group representing the frame the instance appears on | |
| label: the name of the instance group that will be created | |
| **kwargs: additional key:value pairs to be saved as datasets. | |
| Returns: | |
| The h5 group representing this instance. | |
| """ | |
| if label is None: | |
| if self.pred_track_id != -1: | |
| label = f"instance_{self.pred_track_id.item()}" | |
| else: | |
| label = f"instance_{self.gt_track_id.item()}" | |
| instance_group = frame_group.create_group(label) | |
| instance_group.attrs.create("gt_track_id", self.gt_track_id.item()) | |
| instance_group.attrs.create("pred_track_id", self.pred_track_id.item()) | |
| instance_group.attrs.create("track_score", self.track_score) | |
| instance_group.attrs.create("instance_score", self.instance_score) | |
| instance_group.create_dataset("bbox", data=self.bbox.cpu().numpy()) | |
| pose_group = instance_group.create_group("pose") | |
| pose_group.create_dataset("points", data=np.array(list(self.pose.values()))) | |
| pose_group.attrs.create("nodes", list(self.pose.keys())) | |
| pose_group.create_dataset("scores", data=self.point_scores) | |
| for key, value in kwargs.items(): | |
| if "emb" in key: | |
| emb_group = instance_group.require_group("emb") | |
| emb_group.create_dataset(key, data=value) | |
| else: | |
| instance_group.create_dataset(key, data=value) | |
| return instance_group |
Tools
Ruff
268-268: Undefined name
pred_track_id(F821)
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (1)
dreem/io/instance.py (1)
Line range hint
223-244:
Improve exception raising.The function
to_slphas improved error handling by replacinglogger.exception. However, the exception should be raised withraise ... from eto distinguish it from errors in exception handling.- raise RuntimeError(f"Failed to convert to sio.PredictedInstance: {e}") + raise RuntimeError(f"Failed to convert to sio.PredictedInstance: {e}") from e
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- dreem/inference/eval.py (1 hunks)
- dreem/inference/track.py (4 hunks)
- dreem/inference/tracker.py (12 hunks)
- dreem/io/config.py (17 hunks)
- dreem/io/frame.py (14 hunks)
- dreem/io/instance.py (10 hunks)
- dreem/models/gtr_runner.py (6 hunks)
Additional context used
Ruff
dreem/inference/eval.py
7-7:
pathlib.Pathimported but unusedRemove unused import:
pathlib.Path(F401)
12-12:
pytorch_lightningimported but unusedRemove unused import:
pytorch_lightning(F401)
13-13:
torchimported but unusedRemove unused import:
torch(F401)
29-29: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
46-46: f-string without any placeholders
Remove extraneous
fprefix(F541)
49-49: f-string without any placeholders
Remove extraneous
fprefix(F541)
61-61: Local variable
metricsis assigned to but never usedRemove assignment to unused variable
metrics(F841)
dreem/inference/track.py
21-21: Undefined name
dreem(F821)
118-118: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/models/gtr_runner.py
86-86: Undefined name
dreem(F821)
87-87: Undefined name
dreem(F821)
dreem/io/config.py
115-115: Undefined name
GTRRunner(F821)
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
257-257: Undefined name
SleapDataset(F821)
257-257: Undefined name
MicroscopyDataset(F821)
257-257: Undefined name
CellTrackingDataset(F821)
dreem/inference/tracker.py
144-144: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/io/frame.py
63-63: Undefined name
AssociationMatrix(F821)
372-372: Undefined name
Instance(F821)
381-381: Undefined name
Instance(F821)
dreem/io/instance.py
247-247: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
269-269: Undefined name
pred_track_id(F821)
Additional comments not posted (33)
dreem/inference/eval.py (4)
46-46: Remove extraneous f-string prefix.The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Using the following tracker:") + logger.info("Using the following tracker:")Tools
Ruff
46-46: f-string without any placeholders
Remove extraneous
fprefix(F541)
49-49: Remove extraneous f-string prefix.The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Computing the following metrics:") + logger.info("Computing the following metrics:")Tools
Ruff
49-49: f-string without any placeholders
Remove extraneous
fprefix(F541)
61-61: Remove unused variable assignment.The variable
metricsis assigned but never used. Remove the assignment to clean up the code.- metrics = trainer.test(model, dataloader) + trainer.test(model, dataloader)Tools
Ruff
61-61: Local variable
metricsis assigned to but never usedRemove assignment to unused variable
metrics(F841)
64-75: LGTM!The example calls in the main block are clear and helpful.
dreem/inference/track.py (4)
Line range hint
21-36:
LGTM!The
export_trajectoriesfunction is well-implemented and handles the optionalsave_pathparameter correctly.Tools
Ruff
21-21: Undefined name
dreem(F821)
106-106: Remove extraneous f-string prefix.The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Pod Index: {index}") + logger.info("Pod Index: {index}")
118-118: Remove extraneous f-string prefix.The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Using the following tracker:") + logger.info("Using the following tracker:")Tools
Ruff
118-118: f-string without any placeholders
Remove extraneous
fprefix(F541)
4-4: Remove unused import statements.The following import statement is unused and should be removed to clean up the code:
pathlib.Path- from pathlib import PathLikely invalid or redundant comment.
dreem/models/gtr_runner.py (3)
39-46: LGTM!The updates to the
__init__method improve the configurability and type safety of the class.
278-320: LGTM!The
on_test_epoch_endmethod is well-implemented and enhances the functionality of the class.
4-7: Remove unused import statements.The following import statements are unused and should be removed to clean up the code:
torchgc- import torch - import gcLikely invalid or redundant comment.
dreem/io/config.py (7)
Line range hint
19-40: LGTM!The changes to the
__init__method improve type safety and logging.
Line range hint
52-60: LGTM!The changes to the
from_yamlmethod improve type safety and readability.
168-193: LGTM!The changes to the
get_data_pathsmethod improve performance and reduce potential side effects.
Line range hint
257-276: LGTM!The changes to the
get_dataloadermethod improve type safety.Tools
Ruff
257-257: Undefined name
SleapDataset(F821)
257-257: Undefined name
MicroscopyDataset(F821)
257-257: Undefined name
CellTrackingDataset(F821)
Line range hint
370-434: LGTM!The changes to the
get_checkpointingandget_trainermethods improve error handling and type safety.
Line range hint
196-251: Verify the imports of datasets.The method imports
SleapDataset,MicroscopyDataset, andCellTrackingDataset, which are flagged as undefined by static analysis. Ensure that these datasets are correctly imported from the appropriate module.Verification successful
Imports of
SleapDataset,MicroscopyDataset, andCellTrackingDatasetare valid.The classes
SleapDataset,MicroscopyDataset, andCellTrackingDatasetare correctly defined in the codebase.
SleapDatasetis defined asclass SleapDataset(BaseDataset)MicroscopyDatasetis defined asclass MicroscopyDataset(BaseDataset)CellTrackingDatasetis defined asclass CellTrackingDataset(BaseDataset)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the imports of `SleapDataset`, `MicroscopyDataset`, and `CellTrackingDataset`. # Test: Search for the definitions of the datasets. Expect: Valid definitions. rg --type python -A 5 $'class SleapDataset' rg --type python -A 5 $'class MicroscopyDataset' rg --type python -A 5 $'class CellTrackingDataset'Length of output: 239
Script:
#!/bin/bash # Description: Verify the imports of `SleapDataset`, `MicroscopyDataset`, and `CellTrackingDataset`. # Step 1: Find all Python files. fd -e py > python_files.txt # Step 2: Search for the definitions of the datasets in the found Python files. rg -f python_files.txt -A 5 $'class SleapDataset' rg -f python_files.txt -A 5 $'class MicroscopyDataset' rg -f python_files.txt -A 5 $'class CellTrackingDataset'Length of output: 364
Script:
#!/bin/bash # Description: Verify the imports of `SleapDataset`, `MicroscopyDataset`, and `CellTrackingDataset`. # Step 1: Find all Python files and search for the definitions of the datasets in those files. fd -e py -x rg -A 5 'class SleapDataset' {} fd -e py -x rg -A 5 'class MicroscopyDataset' {} fd -e py -x rg -A 5 'class CellTrackingDataset' {}Length of output: 629
Tools
Ruff
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
Line range hint
115-144: Verify the import ofGTRRunner.The method imports
GTRRunner, which is flagged as undefined by static analysis. Ensure thatGTRRunneris correctly imported from the appropriate module.Verification successful
Verify the import path for
GTRRunner.The
GTRRunnerclass is defined indreem/models/gtr_runner.py, so the import statement in theget_gtr_runnermethod should be:from dreem.models.gtr_runner import GTRRunnerPlease ensure that the import statement in the
get_gtr_runnermethod correctly reflects this path.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the import of `GTRRunner`. # Test: Search for the definition of `GTRRunner`. Expect: A valid definition. rg --type python -A 5 $'class GTRRunner'Length of output: 72
Script:
#!/bin/bash # Description: Verify the import of `GTRRunner`. # Test: Search for the definition of `GTRRunner`. Expect: A valid definition. rg --type py -A 5 $'class GTRRunner'Length of output: 337
Tools
Ruff
115-115: Undefined name
GTRRunner(F821)
dreem/inference/tracker.py (3)
Line range hint
27-33: LGTM!The changes to the
__init__method improve type safety and readability.
79-96: LGTM!The addition of the
__repr__method provides a detailed string representation of the tracker instance, which is useful for debugging and logging.
Line range hint
171-353: LGTM!The changes to the
sliding_inferenceand_run_global_trackermethods improve logging and maintainability.dreem/io/frame.py (6)
Line range hint
16-26: LGTM!The changes to the
_to_tensorfunction improve type safety and readability.
Line range hint
101-114: LGTM!The changes to the
tomethod improve type safety and readability.
Line range hint
136-154: LGTM!The changes to the
from_slpmethod improve type safety and readability.
Line range hint
168-195: LGTM!The changes to the
to_slpmethod improve type safety and readability.
205-250: LGTM!The addition of the
to_h5method expands the class's capabilities for data handling by allowing conversion of aFrameinstance to an HDF5 group.
Line range hint
63-339: Verify the imports ofAssociationMatrixandInstance.The property methods for
video,vid_name,instances, andget_traj_scorehave been updated to use union type syntax. Ensure thatAssociationMatrixandInstanceare correctly imported from the appropriate module.Verification successful
Imports for
AssociationMatrixandInstanceare valid.The classes
AssociationMatrixandInstanceare correctly defined indreem/io/association_matrix.pyanddreem/io/instance.pyrespectively.
AssociationMatrix:dreem/io/association_matrix.py:class AssociationMatrixInstance:dreem/io/instance.py:class InstanceScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the imports of `AssociationMatrix` and `Instance`. # Test: Search for the definitions of `AssociationMatrix` and `Instance`. Expect: Valid definitions. rg --type python -A 5 $'class AssociationMatrix' rg --type python -A 5 $'class Instance'Length of output: 152
Script:
#!/bin/bash # Description: Verify the imports of `AssociationMatrix` and `Instance`. # Test: Search for the definitions of `AssociationMatrix` and `Instance`. Expect: Valid definitions. rg -t py -A 5 $'class AssociationMatrix' rg -t py -A 5 $'class Instance'Length of output: 860
dreem/io/instance.py (6)
15-18: LGTM!The function
_to_tensorhas been updated to use the modern type hint syntax with the pipe operator. The change is appropriate and improves readability.
35-36: LGTM!The function
_expand_to_rankhas been updated to use the modern type hint syntax with the pipe operator. The change is appropriate and improves readability.
138-138: LGTM!The function
tohas been updated to use the modern type hint syntax with the pipe operator and the return typeSelf. The change is appropriate and improves readability.
161-165: LGTM!The function
from_slphas been updated to use the modern type hint syntax with the pipe operator and the return typeSelf. The change is appropriate and improves readability.
525-525: LGTM!The function
has_embeddinghas been updated to use the modern type hint syntax with the pipe operator. The change is appropriate and improves readability.
Line range hint
538-554:
LGTM!The function
get_embeddinghas been updated to use the modern type hint syntax with the pipe operator. The change is appropriate and improves readability.
| def to_h5(self, frame_group=h5py.Group, label=None, **kwargs: dict) -> h5py.Group: | ||
| """Convert instance to an h5 group". | ||
|
|
||
| By default we always save: | ||
| - the gt/pred track id | ||
| - bbox | ||
| - centroid | ||
| - pose | ||
| - instance/traj/points score | ||
| Larger arrays (crops/features/embeddings) can be saved by passing as kwargs | ||
|
|
||
| Args: | ||
| frame_group: the h5py group representing the frame the instance appears on | ||
| label: the name of the instance group that will be created | ||
| **kwargs: additional key:value pairs to be saved as datasets. | ||
|
|
||
| Returns: | ||
| The h5 group representing this instance. | ||
| """ | ||
| if label is None: | ||
| if pred_track_id != -1: | ||
| label = f"instance_{self.pred_track_id.item()}" | ||
| else: | ||
| label = f"instance_{self.gt_track_id.item()}" | ||
| instance_group = frame_group.create_group(label) | ||
| instance_group.attrs.create("gt_track_id", self.gt_track_id.item()) | ||
| instance_group.attrs.create("pred_track_id", self.pred_track_id.item()) | ||
| instance_group.attrs.create("track_score", self.track_score) | ||
| instance_group.attrs.create("instance_score", self.instance_score) | ||
|
|
||
| instance_group.create_dataset("bbox", data=self.bbox.cpu().numpy()) | ||
|
|
||
| pose_group = instance_group.create_group("pose") | ||
| pose_group.create_dataset("points", data=np.array(list(self.pose.values()))) | ||
| pose_group.attrs.create("nodes", list(self.pose.keys())) | ||
| pose_group.create_dataset("scores", data=self.point_scores) | ||
|
|
||
| for key, value in kwargs.items(): | ||
| if "emb" in key: | ||
| emb_group = instance_group.require_group("emb") | ||
| emb_group.create_dataset(key, data=value) | ||
| else: | ||
| instance_group.create_dataset(key, data=value) | ||
|
|
||
| return instance_group |
There was a problem hiding this comment.
Fix undefined variable pred_track_id.
The variable pred_track_id is undefined and should be replaced with self.pred_track_id.
- if pred_track_id != -1:
- label = f"instance_{pred_track_id.item()}"
+ if self.pred_track_id != -1:
+ label = f"instance_{self.pred_track_id.item()}"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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def to_h5(self, frame_group=h5py.Group, label=None, **kwargs: dict) -> h5py.Group: | |
| """Convert instance to an h5 group". | |
| By default we always save: | |
| - the gt/pred track id | |
| - bbox | |
| - centroid | |
| - pose | |
| - instance/traj/points score | |
| Larger arrays (crops/features/embeddings) can be saved by passing as kwargs | |
| Args: | |
| frame_group: the h5py group representing the frame the instance appears on | |
| label: the name of the instance group that will be created | |
| **kwargs: additional key:value pairs to be saved as datasets. | |
| Returns: | |
| The h5 group representing this instance. | |
| """ | |
| if label is None: | |
| if pred_track_id != -1: | |
| label = f"instance_{self.pred_track_id.item()}" | |
| else: | |
| label = f"instance_{self.gt_track_id.item()}" | |
| instance_group = frame_group.create_group(label) | |
| instance_group.attrs.create("gt_track_id", self.gt_track_id.item()) | |
| instance_group.attrs.create("pred_track_id", self.pred_track_id.item()) | |
| instance_group.attrs.create("track_score", self.track_score) | |
| instance_group.attrs.create("instance_score", self.instance_score) | |
| instance_group.create_dataset("bbox", data=self.bbox.cpu().numpy()) | |
| pose_group = instance_group.create_group("pose") | |
| pose_group.create_dataset("points", data=np.array(list(self.pose.values()))) | |
| pose_group.attrs.create("nodes", list(self.pose.keys())) | |
| pose_group.create_dataset("scores", data=self.point_scores) | |
| for key, value in kwargs.items(): | |
| if "emb" in key: | |
| emb_group = instance_group.require_group("emb") | |
| emb_group.create_dataset(key, data=value) | |
| else: | |
| instance_group.create_dataset(key, data=value) | |
| return instance_group | |
| if label is None: | |
| if self.pred_track_id != -1: | |
| label = f"instance_{self.pred_track_id.item()}" | |
| else: | |
| label = f"instance_{self.gt_track_id.item()}" | |
| instance_group = frame_group.create_group(label) | |
| instance_group.attrs.create("gt_track_id", self.gt_track_id.item()) | |
| instance_group.attrs.create("pred_track_id", self.pred_track_id.item()) | |
| instance_group.attrs.create("track_score", self.track_score) | |
| instance_group.attrs.create("instance_score", self.instance_score) | |
| instance_group.create_dataset("bbox", data=self.bbox.cpu().numpy()) | |
| pose_group = instance_group.create_group("pose") | |
| pose_group.create_dataset("points", data=np.array(list(self.pose.values()))) | |
| pose_group.attrs.create("nodes", list(self.pose.keys())) | |
| pose_group.create_dataset("scores", data=self.point_scores) | |
| for key, value in kwargs.items(): | |
| if "emb" in key: | |
| emb_group = instance_group.require_group("emb") | |
| emb_group.create_dataset(key, data=value) | |
| else: | |
| instance_group.create_dataset(key, data=value) | |
| return instance_group |
Tools
Ruff
269-269: Undefined name
pred_track_id(F821)
| from pathlib import Path | ||
|
|
||
| import hydra | ||
| import os | ||
| import pandas as pd | ||
| import pytorch_lightning as pl | ||
| import torch | ||
| import sleap_io as sio |
There was a problem hiding this comment.
Remove unused import statements.
The following import statements are unused and should be removed to clean up the code:
pathlib.Pathpytorch_lightning as pltorch
- from pathlib import Path
- import pytorch_lightning as pl
- import torchCommittable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from pathlib import Path | |
| import hydra | |
| import os | |
| import pandas as pd | |
| import pytorch_lightning as pl | |
| import torch | |
| import sleap_io as sio | |
| import hydra | |
| import os | |
| import pandas as pd | |
| import sleap_io as sio |
Tools
Ruff
7-7:
pathlib.Pathimported but unusedRemove unused import:
pathlib.Path(F401)
12-12:
pytorch_lightningimported but unusedRemove unused import:
pytorch_lightning(F401)
13-13:
torchimported but unusedRemove unused import:
torch(F401)
| """ | ||
| eval_cfg = Config(cfg) | ||
|
|
||
| if "checkpoints" in cfg.keys(): |
There was a problem hiding this comment.
Simplify dictionary key check.
Use key in dict instead of key in dict.keys() for checking the existence of a key in a dictionary.
- if "checkpoints" in cfg.keys():
+ if "checkpoints" in cfg: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if "checkpoints" in cfg.keys(): | |
| if "checkpoints" in cfg: |
Tools
Ruff
29-29: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
| lf, tracks = frame.to_slp(tracks, video=video) | ||
| pred_slp.append(lf) | ||
| pred_slp = sio.Labels(pred_slp) | ||
| print(pred_slp) |
There was a problem hiding this comment.
Remove extraneous print statement.
The print statement is unnecessary and should be removed to clean up the code.
- print(pred_slp)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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| print(pred_slp) |
dreem/models/gtr_runner.py
Outdated
| logger.exception( | ||
| f"Failed on frame {frames[0].frame_id} of video {frames[0].video_id}" | ||
| ) | ||
| logger.exception(e) |
There was a problem hiding this comment.
Add missing return statement in case of exception.
The _shared_eval_step method should return None in case of an exception to avoid potential issues.
- raise (e)
+ logger.exception(e)
+ return NoneCommittable suggestion was skipped due to low confidence.
dreem/inference/tracker.py
Outdated
| logger.debug(f"Clearing Queue after tracking") | ||
| self.track_queue.end_tracks() |
There was a problem hiding this comment.
Remove extraneous f prefix.
The f-string is not using any placeholders, so the f prefix is unnecessary.
- logger.debug(f"Clearing Queue after tracking")
+ logger.debug("Clearing Queue after tracking")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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.debug(f"Clearing Queue after tracking") | |
| self.track_queue.end_tracks() | |
| logger.debug("Clearing Queue after tracking") | |
| self.track_queue.end_tracks() |
Tools
Ruff
144-144: f-string without any placeholders
Remove extraneous
fprefix(F541)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- dreem/inference/post_processing.py (4 hunks)
- dreem/io/config.py (17 hunks)
- dreem/models/model_utils.py (4 hunks)
- tests/test_inference.py (1 hunks)
Files skipped from review due to trivial changes (1)
- dreem/inference/post_processing.py
Additional context used
Ruff
dreem/models/model_utils.py
8-8: Undefined name
dreem(F821)
33-33: Undefined name
dreem(F821)
dreem/io/config.py
115-115: Undefined name
GTRRunner(F821)
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
257-257: Undefined name
SleapDataset(F821)
257-257: Undefined name
MicroscopyDataset(F821)
257-257: Undefined name
CellTrackingDataset(F821)
Additional comments not posted (11)
dreem/models/model_utils.py (3)
8-8: LGTM!The type hinting update from
Listtolistis correct and aligns with modern Python practices.Tools
Ruff
8-8: Undefined name
dreem(F821)
174-174: LGTM!The type hinting update for
configto use a union type is correct and aligns with modern Python practices.
33-33: LGTM! But verify the function usage in the codebase.The type hinting update for
query_instancesto use a union type is correct. Ensure that all function calls toget_timesmatch the new signature.Tools
Ruff
33-33: Undefined name
dreem(F821)
tests/test_inference.py (1)
206-207: LGTM!The updated tensor dimensions for
k_boxesandnonk_boxesare correct and likely accommodate new data structures or features.dreem/io/config.py (7)
Line range hint
19-39:
LGTM!The type hinting update for
params_cfgto use a union type is correct. Replacing print statements with logging improves maintainability.
52-52: LGTM!The type hinting update for
params_cfg_pathto use a union type is correct and aligns with modern Python practices.
Line range hint
115-135:
LGTM!The type hinting update for
ckpt_pathto use a union type is correct and aligns with modern Python practices.Tools
Ruff
115-115: Undefined name
GTRRunner(F821)
196-200: LGTM!The type hinting update for
label_filesandvid_filesto use union types is correct and aligns with modern Python practices.Tools
Ruff
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
257-257: LGTM!The type hinting update for
datasetto use union types is correct and aligns with modern Python practices.Tools
Ruff
257-257: Undefined name
SleapDataset(F821)
257-257: Undefined name
MicroscopyDataset(F821)
257-257: Undefined name
CellTrackingDataset(F821)
Line range hint
370-390:
LGTM!The logging update for error handling improves maintainability and error tracking.
407-408: LGTM!The type hinting update for
callbacksandloggerto use union types is correct and aligns with modern Python practices.
There was a problem hiding this comment.
Actionable comments posted: 13
Outside diff range, codebase verification and nitpick comments (4)
docs/cli.md (4)
13-25: Specify the language for the code block.The fenced code block should have a language specified for better syntax highlighting.
```bash usage: dreem-train [-h] [--hydra-help] [--config-dir] [--config-name] [+params_config] [+batch_config]Tools
Markdownlint
13-13: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
34-46: Specify the language for the code block.The fenced code block should have a language specified for better syntax highlighting.
```bash usage: dreem-eval [-h] [--hydra-help] [--config-dir] [--config-name] [+params_config] [+batch_config]Tools
Markdownlint
34-34: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
53-65: Specify the language for the code block.The fenced code block should have a language specified for better syntax highlighting.
```bash usage: dreem-track [-h] [--hydra-help] [--config-dir] [--config-name] [+params_config] [+batch_config]Tools
Markdownlint
53-53: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
74-102: Specify the language for the code block.The fenced code block should have a language specified for better syntax highlighting.
```bash usage: dreem-visualize [-h] [--hydra-help] [+labels_path] [+vid_path] [+save_path] [+annotate.key] [--config-path] [+annotate.color_palette="tab20"] [+annotate.trails=2] [+annotate.boxes=64] [+annotate.names=True] [+annotate.track_scores=0.5] [+annotate.centroids=4] [+annotate.poses=False] [+annotate.fps=30] [+annotate.alpha=0.2]Tools
Markdownlint
74-74: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- docs/cli.md (1 hunks)
- docs/configs/eval.md (1 hunks)
- docs/configs/inference.md (1 hunks)
- docs/usage.md (6 hunks)
Files skipped from review due to trivial changes (1)
- docs/configs/inference.md
Additional context used
LanguageTool
docs/cli.md
[uncategorized] ~104-~104: Possible missing comma found.
Context: ...pecifying each individual+annotate.*param we recommend setting up a visualize con...(AI_HYDRA_LEO_MISSING_COMMA)
docs/configs/eval.md
[style] ~17-~17: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ... inference. *use_vis_feats: (bool) Whether or not to use visual feature extractor. * `ove...(WHETHER)
[style] ~19-~19: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...r assignment. *mult_thresh: (bool) Whether or not to use weight threshold. *decay_time...(WHETHER)
[uncategorized] ~46-~46: Loose punctuation mark.
Context: ...asets/base_dataset.md) args *padding: Anintrepresenting the amount of pad...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...around which a crop will form. *chunk: Whether or not to chunk videos into sma...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~48-~48: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...ound which a crop will form. *chunk: Whether or not to chunk videos into smaller clips to f...(WHETHER)
[uncategorized] ~51-~51: Loose punctuation mark.
Context: ...for training or validation. *n_chunks: Number of chunks to subsample from. Can...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~55-~55: Loose punctuation mark.
Context: ...tracking challenge datasets. ####dir: This section allows you to pass a direc...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~58-~58: Loose punctuation mark.
Context: ... to labels/videos individually *path: The path to the dir where the data is s...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~60-~60: Possible missing comma found.
Context: ... the file extension to search for video files e.g.mp4,.avior.tif. ##### Exa...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~75-~75: Loose punctuation mark.
Context: ...... ... ... ``` ####augmentations: This subsection contains params for al...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~98-~98: Loose punctuation mark.
Context: ... all visible anchors. *handle_missing: how to handle missing single nodes. one...(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~100-~100: Consider adding a comma.
Context: ... missing theanchor. * ifignorethen we use a mask instead of a crop and nan...(IF_THEN_COMMA)
[style] ~101-~101: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... a crop and nan centroids/bboxes. * ifcentroidthen we default to the pose ...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[typographical] ~101-~101: Consider adding a comma.
Context: ...an centroids/bboxes. * ifcentroidthen we default to the pose centroid as the ...(IF_THEN_COMMA)
[uncategorized] ~145-~145: Possible missing comma found.
Context: ... for the dataloaders. For more advanced users see [torch.utils.data.Dataloader](htt...(AI_HYDRA_LEO_MISSING_COMMA)
docs/usage.md
[uncategorized] ~119-~119: Possible missing comma found.
Context: ...nside my/home/aaprasad/dreem_configsdirectory I can call ```bash dreem-train --config...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~139-~139: It seems like one article is redundant in this context.
Context: .../dreem_configs` directory that contains a only a small selection of parameters that I'd like t...(A_RB_A_JJ_NN)
[uncategorized] ~151-~151: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...E_CONFIG_STEM] section.param=value ``` e.g If now I want to override a couple para...(E_G)
[grammar] ~152-~152: Using ‘couple’ without ‘of’ is considered to be informal.
Context: ...ue ``` e.g If now I want to override a couple parameters again, say change the number of attenti...(PLENTY_OF_NOUNS)
[uncategorized] ~183-~183: Possible missing comma found.
Context: ...lly, testing will be done at the end of training assuming you passed in a test dataset. ...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~183-~183: Consider adding a comma after ‘Basically’ for more clarity.
Context: ...for how to set up your evaluation data. Basically we recommend storing your test videos/l...(RB_LY_COMMA)
[misspelling] ~198-~198: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...et up a config file that specifies 1. ackpt_path2. arunner.save_path3. ...(EN_A_VS_AN)
[misspelling] ~199-~199: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...le that specifies 1. ackpt_path2. arunner.save_path3. a list of the [`p...(EN_A_VS_AN)
[uncategorized] ~207-~207: Possible missing comma found.
Context: ...e. ### Step 3 Run Evaluation Just like training we can use the hydra syntax for specify...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~207-~207: A comma may be missing after the conjunctive/linking adverb ‘Thus’.
Context: ...x for specifying arguments via the cli. Thus you can run inference via: ```bash dre...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~220-~220: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ased override rather than the file based but you're more than welcome to do so. In ...(COMMA_COMPOUND_SENTENCE)
[style] ~221-~221: Consider a shorter alternative to avoid wordiness.
Context: ...but you're more than welcome to do so. In order to override params via the CLI, we can use...(IN_ORDER_TO_PREMIUM)
[uncategorized] ~226-~226: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...[CONFIG_STEM] section.param=[VALUE] ``` e.g if I want to set the window size of the...(E_G)
[misspelling] ~232-~232: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ecified. These results will be saved in an hdf5 file saved to the `runner.save_pat...(EN_A_VS_AN)
[style] ~245-~245: In American English, abbreviations like “etc.” require a period.
Context: ...uation metrics, and respective metadata etc will be stored in theattrsof the hd...(ETC_PERIOD)
[uncategorized] ~245-~245: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...w.h5py.org) We also store model inputs (e.g crops, visual encoder features, spatiot...(E_G)
[uncategorized] ~311-~311: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...[CONFIG_STEM] section.param=[VALUE] ``` e.g if I want to set the window size of the...(E_G)
[typographical] ~317-~317: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...pecified in the config it will save to./[OUTDIR]/[VID_NAME].dreem_inference.slp, otherwise it will just save to `./results/[VID_NA...(THUS_SENTENCE)
Markdownlint
docs/cli.md
13-13: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
34-34: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
53-53: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
74-74: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
docs/configs/eval.md
94-94: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
95-95: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
96-96: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
99-99: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
100-100: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
101-101: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
28-28: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
55-55: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
61-61: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
75-75: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
90-90: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
162-162: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
26-26: null
Spaces inside code span elements(MD038, no-space-in-code)
docs/usage.md
219-219: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
233-233: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (6)
docs/configs/eval.md (4)
1-11: LGTM!The section is clear and provides detailed information on inference parameters.
75-89: LGTM!The section is clear and provides detailed information on augmentations.
Tools
LanguageTool
[uncategorized] ~75-~75: Loose punctuation mark.
Context: ...... ... ... ``` ####augmentations: This subsection contains params for al...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
75-75: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
143-157: LGTM!The section is clear and provides detailed information on dataloader configuration.
Tools
LanguageTool
[uncategorized] ~145-~145: Possible missing comma found.
Context: ... for the dataloaders. For more advanced users see [torch.utils.data.Dataloader](htt...(AI_HYDRA_LEO_MISSING_COMMA)
158-175: LGTM!The section is clear and provides detailed information on runner configuration.
Tools
Markdownlint
162-162: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
docs/usage.md (2)
317-317: Fix the incorrect usage of "otherwise".The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
otherwise, it will just save to `./results/[VID_NAME].dreem_inference.slp`.Likely invalid or redundant comment.
Tools
LanguageTool
[typographical] ~317-~317: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...pecified in the config it will save to./[OUTDIR]/[VID_NAME].dreem_inference.slp, otherwise it will just save to `./results/[VID_NA...(THUS_SENTENCE)
198-199: Fix the incorrect article.Use “an” instead of ‘a’ if the following word starts with a vowel sound.
1. an `ckpt_path` 2. an `runner.save_path`Likely invalid or redundant comment.
Tools
LanguageTool
[misspelling] ~198-~198: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...et up a config file that specifies 1. ackpt_path2. arunner.save_path3. ...(EN_A_VS_AN)
[misspelling] ~199-~199: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...le that specifies 1. ackpt_path2. arunner.save_path3. a list of the [`p...(EN_A_VS_AN)
docs/cli.md
Outdated
| +annotate.alpha The opacity of the centroid annotations. | ||
| ``` | ||
|
|
||
| As noted above, instead of specifying each individual `+annotate.*` param we recommend setting up a visualize config (e.g `./configs/visualize.yaml`) which looks like: |
There was a problem hiding this comment.
Fix the missing comma.
There is a possible missing comma in the text.
As noted above, instead of specifying each individual `+annotate.*` param, we recommend setting up a visualize config (e.g `./configs/visualize.yaml`) which looks like: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| As noted above, instead of specifying each individual `+annotate.*` param we recommend setting up a visualize config (e.g `./configs/visualize.yaml`) which looks like: | |
| As noted above, instead of specifying each individual `+annotate.*` param, we recommend setting up a visualize config (e.g `./configs/visualize.yaml`) which looks like: |
Tools
LanguageTool
[uncategorized] ~104-~104: Possible missing comma found.
Context: ...pecifying each individual+annotate.*param we recommend setting up a visualize con...(AI_HYDRA_LEO_MISSING_COMMA)
| #### `dir`: | ||
| This section allows you to pass a directory rather than paths to labels/videos individually | ||
|
|
||
| * `path`: The path to the dir where the data is stored (recommend absolute path) | ||
| * `labels_suffix`: (`str`) containing the file extension to search for labels files. e.g. `.slp`, `.csv`, or `.xml`. | ||
| * `vid_suffix`: (`str`) containing the file extension to search for video files e.g `.mp4`, `.avi` or `.tif`. | ||
| ##### Examples: |
There was a problem hiding this comment.
Fix loose punctuation marks.
There are loose punctuation marks in the text.
#### `dir`:
This section allows you to pass a directory rather than paths to labels/videos individually
* `path`: The path to the dir where the data is stored (recommend absolute path)
* `labels_suffix`: (`str`) containing the file extension to search for labels files. e.g. `.slp`, `.csv`, or `.xml`.
* `vid_suffix`: (`str`) containing the file extension to search for video files e.g `.mp4`, `.avi` or `.tif`.Committable suggestion was skipped due to low confidence.
Tools
LanguageTool
[uncategorized] ~55-~55: Loose punctuation mark.
Context: ...tracking challenge datasets. ####dir: This section allows you to pass a direc...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~58-~58: Loose punctuation mark.
Context: ... to labels/videos individually *path: The path to the dir where the data is s...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~60-~60: Possible missing comma found.
Context: ... the file extension to search for video files e.g.mp4,.avior.tif. ##### Exa...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
55-55: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
61-61: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
| * `window_size`: (`int`) the size of the window used during sliding inference. | ||
| * `use_vis_feats`: (`bool`) Whether or not to use visual feature extractor. | ||
| * `overlap_thresh`: (`float`) the trajectory overlap threshold to be used for assignment. | ||
| * `mult_thresh`: (`bool`) Whether or not to use weight threshold. | ||
| * `decay_time`: (`float`) weight for `decay_time` postprocessing. | ||
| * `iou`: (`str` | `None`) Either `{None, '', "mult" or "max"}`. Whether to use multiplicative or max iou reweighting. | ||
| * `max_center_dist`: (`float`) distance threshold for filtering trajectory score matrix. | ||
| * `persistent_tracking`: (`bool`) whether to keep a buffer across chunks or not. |
There was a problem hiding this comment.
Consider shortening the phrase "whether or not".
Consider shortening the phrase to just ‘whether’, unless you mean ‘regardless of whether’.
* `use_vis_feats`: (`bool`) Whether to use visual feature extractor.
* `mult_thresh`: (`bool`) Whether to use weight threshold.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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * `window_size`: (`int`) the size of the window used during sliding inference. | |
| * `use_vis_feats`: (`bool`) Whether or not to use visual feature extractor. | |
| * `overlap_thresh`: (`float`) the trajectory overlap threshold to be used for assignment. | |
| * `mult_thresh`: (`bool`) Whether or not to use weight threshold. | |
| * `decay_time`: (`float`) weight for `decay_time` postprocessing. | |
| * `iou`: (`str` | `None`) Either `{None, '', "mult" or "max"}`. Whether to use multiplicative or max iou reweighting. | |
| * `max_center_dist`: (`float`) distance threshold for filtering trajectory score matrix. | |
| * `persistent_tracking`: (`bool`) whether to keep a buffer across chunks or not. | |
| * `window_size`: (`int`) the size of the window used during sliding inference. | |
| * `use_vis_feats`: (`bool`) Whether to use visual feature extractor. | |
| * `overlap_thresh`: (`float`) the trajectory overlap threshold to be used for assignment. | |
| * `mult_thresh`: (`bool`) Whether to use weight threshold. | |
| * `decay_time`: (`float`) weight for `decay_time` postprocessing. | |
| * `iou`: (`str` | `None`) Either `{None, '', "mult" or "max"}`. Whether to use multiplicative or max iou reweighting. | |
| * `max_center_dist`: (`float`) distance threshold for filtering trajectory score matrix. | |
| * `persistent_tracking`: (`bool`) whether to keep a buffer across chunks or not. |
Tools
LanguageTool
[style] ~17-~17: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ... inference. *use_vis_feats: (bool) Whether or not to use visual feature extractor. * `ove...(WHETHER)
[style] ~19-~19: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...r assignment. *mult_thresh: (bool) Whether or not to use weight threshold. *decay_time...(WHETHER)
| ``` | ||
|
|
||
| #### Overriding Parameters. | ||
| Because there aren't as many parameters during inference as during training we recommend just using the cli-based override rather than the file based but you're more than welcome to do so. |
There was a problem hiding this comment.
Fix the missing comma.
Use a comma before ‘but’ if it connects two independent clauses.
Because there aren't as many parameters during inference as during training, we recommend just using the cli-based override rather than the file based, but you're more than welcome to do so.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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Because there aren't as many parameters during inference as during training we recommend just using the cli-based override rather than the file based but you're more than welcome to do so. | |
| Because there aren't as many parameters during inference as during training, we recommend just using the cli-based override rather than the file based, but you're more than welcome to do so. |
Tools
LanguageTool
[uncategorized] ~220-~220: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ased override rather than the file based but you're more than welcome to do so. In ...(COMMA_COMPOUND_SENTENCE)
docs/usage.md
Outdated
| dreem-train --config-base=[CONFIG_DIR] --config-name=[CONFIG_STEM] | ||
| ``` | ||
| where `CONFIG_DIR` is the directory that `hydra` should search for the `config.yaml` and `CONFIG_STEM` is the name of the config without the `.yaml` extension. | ||
|
|
||
| e.g. If I have a config file called `base.yaml` inside my `/home/aaprasad/dreem_configs` directory I can call | ||
| ```bash | ||
| python /home/aaprasad/dreem/training/train.py --config-base=/home/aaprasad/dreem_configs --config-name=base | ||
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base |
There was a problem hiding this comment.
Fix the missing comma.
There is a possible missing comma in the text.
e.g. If I have a config file called `base.yaml` inside my `/home/aaprasad/dreem_configs` directory, I can callCommittable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dreem-train --config-base=[CONFIG_DIR] --config-name=[CONFIG_STEM] | |
| ``` | |
| where `CONFIG_DIR` is the directory that `hydra` should search for the `config.yaml` and `CONFIG_STEM` is the name of the config without the `.yaml` extension. | |
| e.g. If I have a config file called `base.yaml` inside my `/home/aaprasad/dreem_configs` directory I can call | |
| ```bash | |
| python /home/aaprasad/dreem/training/train.py --config-base=/home/aaprasad/dreem_configs --config-name=base | |
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base | |
| e.g. If I have a config file called `base.yaml` inside my `/home/aaprasad/dreem_configs` directory, I can call |
Tools
LanguageTool
[uncategorized] ~119-~119: Possible missing comma found.
Context: ...nside my/home/aaprasad/dreem_configsdirectory I can call ```bash dreem-train --config...(AI_HYDRA_LEO_MISSING_COMMA)
docs/usage.md
Outdated
| dreem-track --config-base=[CONFIG_DIR] --config-name=[CONFIG_STEM] section.param=[VALUE] | ||
| ``` | ||
| e.g if I want to set the window size of the tracker to 32 instead of 8 through `tracker.window_size=32` and use a different model saved in `/home/aaprasad/models/new_best.ckpt` I can do: | ||
| ```bash | ||
| python /home/aaprasad/dreem/inference/inference.py --config-base=/home/aaprasad/dreem_configs --config-name=track ckpt_path="/home/aaprasad/models/new_best.ckpt" tracker.window_size=32` | ||
| dreem-track --config-base=/home/aaprasad/dreem_configs --config-name=track ckpt_path="/home/aaprasad/models/new_best.ckpt" tracker.window_size=32` |
There was a problem hiding this comment.
Fix the abbreviation.
The abbreviation “e.g.” requires two periods.
e.g. if I want to set the window size of the tracker to 32 instead of 8 through `tracker.window_size=32` and use a different model saved in `/home/aaprasad/models/new_best.ckpt` I can do: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dreem-track --config-base=[CONFIG_DIR] --config-name=[CONFIG_STEM] section.param=[VALUE] | |
| ``` | |
| e.g if I want to set the window size of the tracker to 32 instead of 8 through `tracker.window_size=32` and use a different model saved in `/home/aaprasad/models/new_best.ckpt` I can do: | |
| ```bash | |
| python /home/aaprasad/dreem/inference/inference.py --config-base=/home/aaprasad/dreem_configs --config-name=track ckpt_path="/home/aaprasad/models/new_best.ckpt" tracker.window_size=32` | |
| dreem-track --config-base=/home/aaprasad/dreem_configs --config-name=track ckpt_path="/home/aaprasad/models/new_best.ckpt" tracker.window_size=32` | |
| e.g. if I want to set the window size of the tracker to 32 instead of 8 through `tracker.window_size=32` and use a different model saved in `/home/aaprasad/models/new_best.ckpt` I can do: |
Tools
LanguageTool
[uncategorized] ~311-~311: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...[CONFIG_STEM] section.param=[VALUE] ``` e.g if I want to set the window size of the...(E_G)
docs/usage.md
Outdated
| dreem-train --config-base=[CONFIG_DIR] --config-name=[BASE_CONFIG_STEM] ++params_config="/path/to/params.yaml" | ||
| ``` | ||
|
|
||
| e.g. If I have a `params_to_override.yaml` file inside my `/home/aaprasad/dreem_configs` directory that contains a only a small selection of parameters that I'd like to override, I can run: | ||
|
|
||
| ```bash | ||
| python /home/aaprasad/dreem/training/train.py --config-base=/home/aaprasad/dreem_configs --config-name=base ++params_config=/home/aaprasad/dreem_configs/params_to_override.yaml | ||
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base ++params_config=/home/aaprasad/dreem_configs/params_to_override.yaml |
There was a problem hiding this comment.
Fix the redundant article.
There is a redundant article in the text.
e.g. If I have a `params_to_override.yaml` file inside my `/home/aaprasad/dreem_configs` directory that contains only a small selection of parameters that I'd like to override, I can run: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dreem-train --config-base=[CONFIG_DIR] --config-name=[BASE_CONFIG_STEM] ++params_config="/path/to/params.yaml" | |
| ``` | |
| e.g. If I have a `params_to_override.yaml` file inside my `/home/aaprasad/dreem_configs` directory that contains a only a small selection of parameters that I'd like to override, I can run: | |
| ```bash | |
| python /home/aaprasad/dreem/training/train.py --config-base=/home/aaprasad/dreem_configs --config-name=base ++params_config=/home/aaprasad/dreem_configs/params_to_override.yaml | |
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base ++params_config=/home/aaprasad/dreem_configs/params_to_override.yaml | |
| dreem-train --config-base=[CONFIG_DIR] --config-name=[BASE_CONFIG_STEM] ++params_config="/path/to/params.yaml" | |
| ``` | |
| e.g. If I have a `params_to_override.yaml` file inside my `/home/aaprasad/dreem_configs` directory that contains only a small selection of parameters that I'd like to override, I can run: | |
| ```bash | |
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base ++params_config=/home/aaprasad/dreem_configs/params_to_override.yaml |
Tools
LanguageTool
[grammar] ~139-~139: It seems like one article is redundant in this context.
Context: .../dreem_configs` directory that contains a only a small selection of parameters that I'd like t...(A_RB_A_JJ_NN)
docs/usage.md
Outdated
| dreem-train --config-base=[CONFIG_DIR] --config-name=[BASE_CONFIG_STEM] section.param=value | ||
| ``` | ||
|
|
||
| e.g If now I want to override a couple parameters again, say change the number of attention heads and change the name of this run in my logger, I can pass `model.head=3` and `logger.name="test_nheads=3"` into | ||
|
|
||
| ```bash | ||
| python /home/aaprasad/dreem/training/train.py --config-base=/home/aaprasad/dreem_configs --config-name=base model.nhead=3 logger.name="test_nheads=3" | ||
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base model.nhead=3 logger.name="test_nheads=3" |
There was a problem hiding this comment.
Fix the abbreviation and informal usage.
The abbreviation “e.g.” requires two periods, and using ‘couple’ without ‘of’ is considered informal.
e.g. If now I want to override a couple of parameters again, say change the number of attention heads and change the name of this run in my logger, I can pass `model.head=3` and `logger.name="test_nheads=3"` into 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dreem-train --config-base=[CONFIG_DIR] --config-name=[BASE_CONFIG_STEM] section.param=value | |
| ``` | |
| e.g If now I want to override a couple parameters again, say change the number of attention heads and change the name of this run in my logger, I can pass `model.head=3` and `logger.name="test_nheads=3"` into | |
| ```bash | |
| python /home/aaprasad/dreem/training/train.py --config-base=/home/aaprasad/dreem_configs --config-name=base model.nhead=3 logger.name="test_nheads=3" | |
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base model.nhead=3 logger.name="test_nheads=3" | |
| dreem-train --config-base=[CONFIG_DIR] --config-name=[BASE_CONFIG_STEM] section.param=value | |
| ``` | |
| e.g. If now I want to override a couple of parameters again, say change the number of attention heads and change the name of this run in my logger, I can pass `model.head=3` and `logger.name="test_nheads=3"` into | |
| ```bash | |
| dreem-train --config-base=/home/aaprasad/dreem_configs --config-name=base model.nhead=3 logger.name="test_nheads=3" |
Tools
LanguageTool
[uncategorized] ~151-~151: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...E_CONFIG_STEM] section.param=value ``` e.g If now I want to override a couple para...(E_G)
[grammar] ~152-~152: Using ‘couple’ without ‘of’ is considered to be informal.
Context: ...ue ``` e.g If now I want to override a couple parameters again, say change the number of attenti...(PLENTY_OF_NOUNS)
| Normally, testing will be done at the end of training assuming you passed in a test dataset. However, if you'd like to test your model on another dataset, we provide a way to do so here. Please [see above](./usage.md#step-1-generate-ground-truth-data) for how to set up your evaluation data. Basically we recommend storing your test videos/labels files as so to make things easier: | ||
| dataset_name/ | ||
| test/ | ||
| vid_1.{VID_EXTENSION} | ||
| vid_1.{slp, csv, xml} | ||
| . | ||
| . | ||
| . | ||
| vid_n.{slp, csv, xml} | ||
| vid_n.{slp, csv, xml} | ||
| Once you have your dataset set up you can move on to setting up your config! |
There was a problem hiding this comment.
Fix the missing comma.
There is a possible missing comma in the text.
Basically, we recommend storing your test videos/labels files as so to make things easier: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Normally, testing will be done at the end of training assuming you passed in a test dataset. However, if you'd like to test your model on another dataset, we provide a way to do so here. Please [see above](./usage.md#step-1-generate-ground-truth-data) for how to set up your evaluation data. Basically we recommend storing your test videos/labels files as so to make things easier: | |
| dataset_name/ | |
| test/ | |
| vid_1.{VID_EXTENSION} | |
| vid_1.{slp, csv, xml} | |
| . | |
| . | |
| . | |
| vid_n.{slp, csv, xml} | |
| vid_n.{slp, csv, xml} | |
| Once you have your dataset set up you can move on to setting up your config! | |
| Normally, testing will be done at the end of training assuming you passed in a test dataset. However, if you'd like to test your model on another dataset, we provide a way to do so here. Please [see above](./usage.md#step-1-generate-ground-truth-data) for how to set up your evaluation data. Basically, we recommend storing your test videos/labels files as so to make things easier: | |
| dataset_name/ | |
| test/ | |
| vid_1.{VID_EXTENSION} | |
| vid_1.{slp, csv, xml} | |
| . | |
| . | |
| . | |
| vid_n.{slp, csv, xml} | |
| vid_n.{slp, csv, xml} | |
| Once you have your dataset set up you can move on to setting up your config! |
Tools
LanguageTool
[uncategorized] ~183-~183: Possible missing comma found.
Context: ...lly, testing will be done at the end of training assuming you passed in a test dataset. ...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~183-~183: Consider adding a comma after ‘Basically’ for more clarity.
Context: ...for how to set up your evaluation data. Basically we recommend storing your test videos/l...(RB_LY_COMMA)
| This will run inference on the videos/detections you specified in the `dataset.test_dataset` section of the config and compute the pymotmetrics for the videos you specified. These results will be saved in an hdf5 file saved to the `runner.save_path` argument. It will be structured as so: | ||
| ``` | ||
| results_h5/ | ||
| vid_1_file_name/ | ||
| clip_1/ | ||
| frame_0/ | ||
| instance_0/ | ||
| instance_1/ | ||
| ... | ||
| instance_n/ | ||
| traj_scores | ||
| asso_output | ||
| ``` | ||
| The evaluation metrics, and respective metadata etc will be stored in the `attrs` of the hdf5 file. If you aren't familiar with opening, reading and manipulating hdf5 files, we recommend checking out the docs for [`h5py`](https://www.h5py.org) We also store model inputs (e.g crops, visual encoder features, spatiotemporal embeddings) in failure cases (identity switches) |
There was a problem hiding this comment.
Fix the incorrect article and stylistic issue.
Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound. In American English, abbreviations like “etc.” require a period.
These results will be saved in a hdf5 file saved to the `runner.save_path` argument. It will be structured as so:
The evaluation metrics, and respective metadata, etc. will be stored in the `attrs` of the hdf5 file. If you aren't familiar with opening, reading and manipulating hdf5 files, we recommend checking out the docs for [`h5py`](https://www.h5py.org) We also store model inputs (e.g. crops, visual encoder features, spatiotemporal embeddings) in failure cases (identity switches)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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This will run inference on the videos/detections you specified in the `dataset.test_dataset` section of the config and compute the pymotmetrics for the videos you specified. These results will be saved in an hdf5 file saved to the `runner.save_path` argument. It will be structured as so: | |
| ``` | |
| results_h5/ | |
| vid_1_file_name/ | |
| clip_1/ | |
| frame_0/ | |
| instance_0/ | |
| instance_1/ | |
| ... | |
| instance_n/ | |
| traj_scores | |
| asso_output | |
| ``` | |
| The evaluation metrics, and respective metadata etc will be stored in the `attrs` of the hdf5 file. If you aren't familiar with opening, reading and manipulating hdf5 files, we recommend checking out the docs for [`h5py`](https://www.h5py.org) We also store model inputs (e.g crops, visual encoder features, spatiotemporal embeddings) in failure cases (identity switches) | |
| This will run inference on the videos/detections you specified in the `dataset.test_dataset` section of the config and compute the pymotmetrics for the videos you specified. These results will be saved in a hdf5 file saved to the `runner.save_path` argument. It will be structured as so: | |
| ``` | |
| results_h5/ | |
| vid_1_file_name/ | |
| clip_1/ | |
| frame_0/ | |
| instance_0/ | |
| instance_1/ | |
| ... | |
| instance_n/ | |
| traj_scores | |
| asso_output | |
| ``` | |
| The evaluation metrics, and respective metadata, etc. will be stored in the `attrs` of the hdf5 file. If you aren't familiar with opening, reading and manipulating hdf5 files, we recommend checking out the docs for [`h5py`](https://www.h5py.org) We also store model inputs (e.g. crops, visual encoder features, spatiotemporal embeddings) in failure cases (identity switches) |
Tools
LanguageTool
[misspelling] ~232-~232: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ecified. These results will be saved in an hdf5 file saved to the `runner.save_pat...(EN_A_VS_AN)
[style] ~245-~245: In American English, abbreviations like “etc.” require a period.
Context: ...uation metrics, and respective metadata etc will be stored in theattrsof the hd...(ETC_PERIOD)
[uncategorized] ~245-~245: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...w.h5py.org) We also store model inputs (e.g crops, visual encoder features, spatiot...(E_G)
Markdownlint
233-233: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
… of `typing.Union` and referring to standard collections directly rather than `typing.Collection`
… number of points
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range, codebase verification and nitpick comments (3)
docs/cli.md (2)
34-46: Specify language for code blocks.Add the language identifier
shellto the fenced code block for better readability.- ``` + ```shellTools
Markdownlint
34-34: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Line range hint
52-64:
Specify language for code blocks.Add the language identifier
shellto the fenced code block for better readability.- ``` + ```shellTools
Markdownlint
34-34: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
53-53: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
docs/configs/eval.md (1)
16-23: Consider shortening the phrase "whether or not".Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
- * `use_vis_feats`: (`bool`) Whether or not to use visual feature extractor. - * `mult_thresh`: (`bool`) Whether or not to use weight threshold. + * `use_vis_feats`: (`bool`) Whether to use visual feature extractor. + * `mult_thresh`: (`bool`) Whether to use weight threshold.Tools
LanguageTool
[style] ~17-~17: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ... inference. *use_vis_feats: (bool) Whether or not to use visual feature extractor. * `ove...(WHETHER)
[style] ~19-~19: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...r assignment. *mult_thresh: (bool) Whether or not to use weight threshold. *decay_time...(WHETHER)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- docs/cli.md (1 hunks)
- docs/configs/eval.md (1 hunks)
- docs/configs/inference.md (1 hunks)
- docs/usage.md (1 hunks)
- dreem/inference/configs/eval.yaml (1 hunks)
- dreem/inference/eval.py (1 hunks)
- dreem/inference/post_processing.py (1 hunks)
- dreem/inference/track.py (3 hunks)
- dreem/inference/track_queue.py (1 hunks)
- dreem/inference/tracker.py (2 hunks)
- dreem/io/config.py (12 hunks)
- dreem/io/frame.py (5 hunks)
- dreem/io/instance.py (3 hunks)
- dreem/models/gtr_runner.py (7 hunks)
- dreem/models/model_utils.py (1 hunks)
- pyproject.toml (1 hunks)
- tests/test_inference.py (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- docs/configs/inference.md
- dreem/models/model_utils.py
- pyproject.toml
- tests/test_inference.py
Additional context used
yamllint
dreem/inference/configs/eval.yaml
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 24-24: no new line character at the end of file
(new-line-at-end-of-file)
[error] 24-24: trailing spaces
(trailing-spaces)
Ruff
dreem/inference/eval.py
7-7:
pathlib.Pathimported but unusedRemove unused import:
pathlib.Path(F401)
12-12:
pytorch_lightningimported but unusedRemove unused import:
pytorch_lightning(F401)
13-13:
torchimported but unusedRemove unused import:
torch(F401)
29-29: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
46-46: f-string without any placeholders
Remove extraneous
fprefix(F541)
49-49: f-string without any placeholders
Remove extraneous
fprefix(F541)
61-61: Local variable
metricsis assigned to but never usedRemove assignment to unused variable
metrics(F841)
dreem/inference/track.py
118-118: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/inference/track_queue.py
9-9: Redefinition of unused
devicefrom line 5Remove definition:
device(F811)
dreem/io/config.py
115-115: Undefined name
GTRRunner(F821)
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
dreem/io/instance.py
269-269: Undefined name
pred_track_id(F821)
Markdownlint
docs/cli.md
34-34: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
docs/configs/eval.md
94-94: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
95-95: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
96-96: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
99-99: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
100-100: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
101-101: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
28-28: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
55-55: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
61-61: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
75-75: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
90-90: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
162-162: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
26-26: null
Spaces inside code span elements(MD038, no-space-in-code)
docs/usage.md
219-219: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
233-233: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
LanguageTool
docs/configs/eval.md
[style] ~17-~17: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ... inference. *use_vis_feats: (bool) Whether or not to use visual feature extractor. * `ove...(WHETHER)
[style] ~19-~19: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...r assignment. *mult_thresh: (bool) Whether or not to use weight threshold. *decay_time...(WHETHER)
[uncategorized] ~46-~46: Loose punctuation mark.
Context: ...asets/base_dataset.md) args *padding: Anintrepresenting the amount of pad...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...around which a crop will form. *chunk: Whether or not to chunk videos into sma...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~48-~48: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...ound which a crop will form. *chunk: Whether or not to chunk videos into smaller clips to f...(WHETHER)
[uncategorized] ~51-~51: Loose punctuation mark.
Context: ...for training or validation. *n_chunks: Number of chunks to subsample from. Can...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~55-~55: Loose punctuation mark.
Context: ...tracking challenge datasets. ####dir: This section allows you to pass a direc...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~58-~58: Loose punctuation mark.
Context: ... to labels/videos individually *path: The path to the dir where the data is s...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~75-~75: Loose punctuation mark.
Context: ...... ... ... ``` ####augmentations: This subsection contains params for al...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~98-~98: Loose punctuation mark.
Context: ... all visible anchors. *handle_missing: how to handle missing single nodes. one...(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~100-~100: Consider adding a comma.
Context: ... missing theanchor. * ifignorethen we use a mask instead of a crop and nan...(IF_THEN_COMMA)
[style] ~101-~101: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... a crop and nan centroids/bboxes. * ifcentroidthen we default to the pose ...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[typographical] ~101-~101: Consider adding a comma.
Context: ...an centroids/bboxes. * ifcentroidthen we default to the pose centroid as the ...(IF_THEN_COMMA)
[uncategorized] ~145-~145: Possible missing comma found.
Context: ... for the dataloaders. For more advanced users see [torch.utils.data.Dataloader](htt...(AI_HYDRA_LEO_MISSING_COMMA)
docs/usage.md
[uncategorized] ~183-~183: Possible missing comma found.
Context: ...lly, testing will be done at the end of training assuming you passed in a test dataset. ...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~183-~183: Consider adding a comma after ‘Basically’ for more clarity.
Context: ...for how to set up your evaluation data. Basically we recommend storing your test videos/l...(RB_LY_COMMA)
[misspelling] ~198-~198: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...et up a config file that specifies 1. ackpt_path2. arunner.save_path3. ...(EN_A_VS_AN)
[misspelling] ~199-~199: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...le that specifies 1. ackpt_path2. arunner.save_path3. a list of the [`p...(EN_A_VS_AN)
[uncategorized] ~207-~207: Possible missing comma found.
Context: ...e. ### Step 3 Run Evaluation Just like training we can use the hydra syntax for specify...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~207-~207: A comma may be missing after the conjunctive/linking adverb ‘Thus’.
Context: ...x for specifying arguments via the cli. Thus you can run inference via: ```bash dre...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~220-~220: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ased override rather than the file based but you're more than welcome to do so. In ...(COMMA_COMPOUND_SENTENCE)
[style] ~221-~221: Consider a shorter alternative to avoid wordiness.
Context: ...but you're more than welcome to do so. In order to override params via the CLI, we can use...(IN_ORDER_TO_PREMIUM)
[uncategorized] ~226-~226: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...[CONFIG_STEM] section.param=[VALUE] ``` e.g if I want to set the window size of the...(E_G)
[misspelling] ~232-~232: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ecified. These results will be saved in an hdf5 file saved to the `runner.save_pat...(EN_A_VS_AN)
[style] ~245-~245: In American English, abbreviations like “etc.” require a period.
Context: ...uation metrics, and respective metadata etc will be stored in theattrsof the hd...(ETC_PERIOD)
[uncategorized] ~245-~245: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...w.h5py.org) We also store model inputs (e.g crops, visual encoder features, spatiot...(E_G)
Additional comments not posted (35)
dreem/inference/configs/eval.yaml (1)
22-24: Fix trailing spaces and add a new line character at the end of the file.Trailing spaces and missing new line character at the end of the file should be fixed.
- - + +Tools
yamllint
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 24-24: no new line character at the end of file
(new-line-at-end-of-file)
[error] 24-24: trailing spaces
(trailing-spaces)
dreem/inference/eval.py (5)
7-14: Remove unused import statements.The following import statements are unused and should be removed to clean up the code:
pathlib.Pathpytorch_lightning as pltorch- from pathlib import Path - import pytorch_lightning as pl - import torchTools
Ruff
7-7:
pathlib.Pathimported but unusedRemove unused import:
pathlib.Path(F401)
12-12:
pytorch_lightningimported but unusedRemove unused import:
pytorch_lightning(F401)
13-13:
torchimported but unusedRemove unused import:
torch(F401)
29-29: Simplify dictionary key check.Use
key in dictinstead ofkey in dict.keys()for checking the existence of a key in a dictionary.- if "checkpoints" in cfg.keys(): + if "checkpoints" in cfg:Tools
Ruff
29-29: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
46-46: Remove extraneous f-string prefix.The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Using the following tracker:") + logger.info("Using the following tracker:")Tools
Ruff
46-46: f-string without any placeholders
Remove extraneous
fprefix(F541)
49-49: Remove extraneous f-string prefix.The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Computing the following metrics:") + logger.info("Computing the following metrics:")Tools
Ruff
49-49: f-string without any placeholders
Remove extraneous
fprefix(F541)
61-61: Remove unused variable assignment.The variable
metricsis assigned but never used. Remove the assignment to clean up the code.- metrics = trainer.test(model, dataloader) + trainer.test(model, dataloader)Tools
Ruff
61-61: Local variable
metricsis assigned to but never usedRemove assignment to unused variable
metrics(F841)
dreem/inference/track.py (2)
86-86: Remove extraneous print statement.The print statement is unnecessary and should be removed to clean up the code.
- print(pred_slp)
118-118: Remove extraneous f-string prefix.The f-string prefix is unnecessary here as there are no placeholders.
- logger.info(f"Using the following tracker:") + logger.info("Using the following tracker:")Tools
Ruff
118-118: f-string without any placeholders
Remove extraneous
fprefix(F541)
dreem/inference/post_processing.py (5)
Line range hint
15-15:
Use consistent type annotations.The type annotations for
k_boxes,nonk_boxes, andid_indshave been updated to use| None. Ensure this is consistent with the rest of the codebase.
146-147: Ensure correct computation of center points.The computation of
k_ctandk_sseems correct for the new tensor shapes. Ensure the shapes ofk_boxesare as expected.
149-149: Ensure correct computation of non-key center points.The computation of
nonk_ctseems correct for the new tensor shapes. Ensure the shapes ofnonk_boxesare as expected.
151-153: Verify distance calculation with broadcasting.The distance calculation now uses broadcasting to handle the new tensor shapes. Ensure this operation is efficient and correct.
158-158: Ensure correct filtering based on distance.The filtering based on
max_center_distseems correct. Ensure thevalid_assnmatrix is computed as intended.docs/cli.md (2)
29-31: Ensure accurate command description.The description for
dreem-evalshould accurately reflect its functionality for evaluating trained models.
48-48: Ensure accurate command description.The description for
dreem-trackshould accurately reflect its functionality for tracking using pretrained models.dreem/inference/track_queue.py (3)
Line range hint
55-74:
LGTM! Improved type hinting.The updated type hint for the
track_idparameter enhances type safety and clarity.Tools
Ruff
9-9: Redefinition of unused
devicefrom line 5Remove definition:
device(F811)
Line range hint
131-164:
LGTM! Improved type hinting.The updated type hints for the
track_idsanddeviceparameters enhance type safety and clarity.Tools
Ruff
9-9: Redefinition of unused
devicefrom line 5Remove definition:
device(F811)
Line range hint
76-129:
LGTM! Improved logging practices.The updated logging statements to use
logger.debugenhance the control over log levels and formats.Tools
Ruff
9-9: Redefinition of unused
devicefrom line 5Remove definition:
device(F811)
dreem/models/gtr_runner.py (3)
Line range hint
35-59:
LGTM! Improved type hinting and configurability.The updated type hints enhance type safety and clarity. The new
test_save_pathparameter improves configurability.
Line range hint
86-101:
LGTM! Improved type hinting.The updated type hint for the
query_instancesparameter enhances type safety and clarity.
278-320: LGTM! Enhanced functionality.The new
on_test_epoch_endmethod significantly enhances functionality by enabling structured storage of test results usingpandasandh5py.dreem/io/config.py (3)
Line range hint
115-144:
LGTM! Improved flexibility.The updated signature to accept an optional
ckpt_pathparameter enhances flexibility by allowing users to override the checkpoint path directly.Tools
Ruff
115-115: Undefined name
GTRRunner(F821)
Line range hint
196-239:
LGTM! Improved flexibility.The updated signature to include optional parameters for
label_filesandvid_filesenhances flexibility by providing greater control over dataset retrieval.Tools
Ruff
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
423-438: LGTM! Improved type hinting.The updated type hints for the
callbacksandloggerparameters enhance type safety and clarity.dreem/inference/tracker.py (3)
79-95: LGTM!The
__repr__method is well-implemented and provides a detailed string representation of theTrackerobject.
Line range hint
14-36:
LGTM!The updated type hints for
decay_time,iou, andmax_center_distparameters improve type safety and code clarity.
301-301: LGTM! But verify the impact of the change.The change to assign
asso_outputfromasso_matrix[-1]seems like a refinement in the logic for handling tracking outputs.However, ensure that this change does not introduce any unintended side effects.
docs/usage.md (1)
198-199: Fix the incorrect article.Use “an” instead of ‘a’ if the following word starts with a vowel sound.
- 1. a `ckpt_path` - 2. a `runner.save_path` + 1. an `ckpt_path` + 2. an `runner.save_path`Likely invalid or redundant comment.
Tools
LanguageTool
[misspelling] ~198-~198: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...et up a config file that specifies 1. ackpt_path2. arunner.save_path3. ...(EN_A_VS_AN)
[misspelling] ~199-~199: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...le that specifies 1. ackpt_path2. arunner.save_path3. a list of the [`p...(EN_A_VS_AN)
dreem/io/frame.py (4)
Line range hint
14-23:
LGTM!The updated type annotations improve readability and modernize the code.
Line range hint
64-77:
LGTM!The updated type annotations improve readability and modernize the code. The use of
Selfenhances type safety.
Line range hint
79-101:
LGTM!The updated type annotations improve readability and modernize the code. The use of
Selfenhances type safety.
Line range hint
168-195:
LGTM!The updated type annotations improve readability and modernize the code. The new
videoparameter enhances the function's flexibility.dreem/io/instance.py (3)
8-8: Import statement forh5pyadded.The import statement for
h5pyis necessary for the newto_h5method.
249-250: New method added for HDF5 conversion.The
to_h5method is a valuable addition that converts anInstanceto an HDF5 group. The method is well-documented and the implementation appears sound.
273-293: Ensure robustness of theto_h5method.The method correctly handles various attributes and additional key-value pairs. Ensure that all necessary attributes are included and correctly processed.
| The h5 group representing this instance. | ||
| """ | ||
| if label is None: | ||
| if pred_track_id != -1: |
There was a problem hiding this comment.
Fix undefined variable pred_track_id.
The variable pred_track_id is undefined and should be replaced with self.pred_track_id.
- if pred_track_id != -1:
- label = f"instance_{pred_track_id.item()}"
+ if self.pred_track_id != -1:
+ label = f"instance_{self.pred_track_id.item()}"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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if pred_track_id != -1: | |
| if self.pred_track_id != -1: | |
| label = f"instance_{self.pred_track_id.item()}" |
Tools
Ruff
269-269: Undefined name
pred_track_id(F821)
| norm_dist = dist / (k_s[:, None, :] + 1e-8) | ||
| norm_dist = dist.mean(axis=-1) # n_k x Np |
There was a problem hiding this comment.
Fix the normalization step.
The normalization step should compute the mean of norm_dist instead of dist.
- norm_dist = dist.mean(axis=-1) # n_k x Np
+ norm_dist = norm_dist.mean(axis=-1) # n_k x NpCommittable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| norm_dist = dist / (k_s[:, None, :] + 1e-8) | |
| norm_dist = dist.mean(axis=-1) # n_k x Np | |
| norm_dist = dist / (k_s[:, None, :] + 1e-8) | |
| norm_dist = norm_dist.mean(axis=-1) # n_k x Np |
| +params_config Path to .yaml file containing subset of params to override | ||
| +batch_config Path to .csv file where each row indicates params to override for a single task in a batch job | ||
| ``` | ||
| See [the usage guide](./usage.md#step-2-set-up-config) for a more in-depth explanation on how to use `dreem-track` and see [the inference config walkthrough](./configs/inference.md) for all available parameters. |
There was a problem hiding this comment.
Fix the usage guide link.
The link to the usage guide should refer to dreem-eval instead of dreem-track.
- See [the usage guide](./usage.md#step-2-set-up-config) for a more in-depth explanation on how to use `dreem-track` and see [the inference config walkthrough](./configs/inference.md) for all available parameters.
+ See [the usage guide](./usage.md#step-2-set-up-config) for a more in-depth explanation on how to use `dreem-eval` and see [the inference config walkthrough](./configs/inference.md) for all available parameters.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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| See [the usage guide](./usage.md#step-2-set-up-config) for a more in-depth explanation on how to use `dreem-track` and see [the inference config walkthrough](./configs/inference.md) for all available parameters. | |
| See [the usage guide](./usage.md#step-2-set-up-config) for a more in-depth explanation on how to use `dreem-eval` and see [the inference config walkthrough](./configs/inference.md) for all available parameters. |
| ... | ||
| ... | ||
| ``` | ||
| #### `augmentations`: |
There was a problem hiding this comment.
Fix loose punctuation marks.
There are loose punctuation marks in the text.
- #### `augmentations`:
+ #### `augmentations`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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### `augmentations`: | |
| #### `augmentations` |
Tools
LanguageTool
[uncategorized] ~75-~75: Loose punctuation mark.
Context: ...... ... ... ``` ####augmentations: This subsection contains params for al...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
75-75: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
| * `padding`: An `int` representing the amount of padding to be added to each side of the bounding box size | ||
| * `crop_size`: (`int`|`tuple`) the size of the bounding box around which a crop will form. | ||
| * `chunk`: Whether or not to chunk videos into smaller clips to feed to model | ||
| * `clip_length`: the number of frames in each chunk | ||
| * `mode`: `train` or `val`. Determines whether this dataset is used for training or validation. | ||
| * `n_chunks`: Number of chunks to subsample from. Can either a fraction of the dataset (ie `(0,1.0]`) or number of chunks | ||
| * `seed`: set a seed for reproducibility | ||
| * `gt_list`: An optional path to .txt file containing ground truth for cell tracking challenge datasets. | ||
|
|
There was a problem hiding this comment.
Fix loose punctuation marks.
There are loose punctuation marks in the text.
- * `padding`: An `int` representing the amount of padding to be added to each side of the bounding box size
- * `chunk`: Whether or not to chunk videos into smaller clips to feed to model
+ * `padding`: An `int` representing the amount of padding to be added to each side of the bounding box size.
+ * `chunk`: Whether to chunk videos into smaller clips to feed to model.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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * `padding`: An `int` representing the amount of padding to be added to each side of the bounding box size | |
| * `crop_size`: (`int`|`tuple`) the size of the bounding box around which a crop will form. | |
| * `chunk`: Whether or not to chunk videos into smaller clips to feed to model | |
| * `clip_length`: the number of frames in each chunk | |
| * `mode`: `train` or `val`. Determines whether this dataset is used for training or validation. | |
| * `n_chunks`: Number of chunks to subsample from. Can either a fraction of the dataset (ie `(0,1.0]`) or number of chunks | |
| * `seed`: set a seed for reproducibility | |
| * `gt_list`: An optional path to .txt file containing ground truth for cell tracking challenge datasets. | |
| * `padding`: An `int` representing the amount of padding to be added to each side of the bounding box size. | |
| * `crop_size`: (`int`|`tuple`) the size of the bounding box around which a crop will form. | |
| * `chunk`: Whether to chunk videos into smaller clips to feed to model. | |
| * `clip_length`: the number of frames in each chunk | |
| * `mode`: `train` or `val`. Determines whether this dataset is used for training or validation. | |
| * `n_chunks`: Number of chunks to subsample from. Can either a fraction of the dataset (ie `(0,1.0]`) or number of chunks | |
| * `seed`: set a seed for reproducibility | |
| * `gt_list`: An optional path to .txt file containing ground truth for cell tracking challenge datasets. |
Tools
LanguageTool
[uncategorized] ~46-~46: Loose punctuation mark.
Context: ...asets/base_dataset.md) args *padding: Anintrepresenting the amount of pad...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...around which a crop will form. *chunk: Whether or not to chunk videos into sma...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~48-~48: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...ound which a crop will form. *chunk: Whether or not to chunk videos into smaller clips to f...(WHETHER)
[uncategorized] ~51-~51: Loose punctuation mark.
Context: ...for training or validation. *n_chunks: Number of chunks to subsample from. Can...(UNLIKELY_OPENING_PUNCTUATION)
| ### [`MicroscopyDataset`](../reference/dreem/datasets/microscopy_dataset.md) | ||
| * `videos`: (`list[str | list[str]]`) paths to raw microscopy videos | ||
| * `tracks`: (`list[str]`) paths to trackmate gt labels (either `.xml` or `.csv`) | ||
| * `source`: file format of gt labels based on label generator. Either `"trackmate"` or `"isbi"`. | ||
| ### [`CellTrackingDataset`](../reference/dreem/datasets/cell_tracking_dataset.md) | ||
| * `raw_images`: (`list[list[str] | list[list[str]]]`) paths to raw microscopy images | ||
| * `gt_images`: (`list[list[str] | list[list[str]]]`) paths to gt label images | ||
| * `gt_list`: (`list[str]`) An optional path to .txt file containing gt ids stored in cell | ||
| tracking challenge format: `"track_id", "start_frame", | ||
| "end_frame", "parent_id"` | ||
| ### `dataset` Examples |
There was a problem hiding this comment.
Fix loose punctuation marks and improve readability.
There are loose punctuation marks in the text. Consider adding commas and rephrasing for better readability.
- * `gt_list`: (`list[str]`) An optional path to .txt file containing gt ids stored in cell
- tracking challenge format: `"track_id", "start_frame",
- "end_frame", "parent_id"`
+ * `gt_list`: (`list[str]`) An optional path to .txt file containing gt ids stored in cell tracking challenge format: `"track_id", "start_frame", "end_frame", "parent_id"`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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### [`MicroscopyDataset`](../reference/dreem/datasets/microscopy_dataset.md) | |
| * `videos`: (`list[str | list[str]]`) paths to raw microscopy videos | |
| * `tracks`: (`list[str]`) paths to trackmate gt labels (either `.xml` or `.csv`) | |
| * `source`: file format of gt labels based on label generator. Either `"trackmate"` or `"isbi"`. | |
| ### [`CellTrackingDataset`](../reference/dreem/datasets/cell_tracking_dataset.md) | |
| * `raw_images`: (`list[list[str] | list[list[str]]]`) paths to raw microscopy images | |
| * `gt_images`: (`list[list[str] | list[list[str]]]`) paths to gt label images | |
| * `gt_list`: (`list[str]`) An optional path to .txt file containing gt ids stored in cell | |
| tracking challenge format: `"track_id", "start_frame", | |
| "end_frame", "parent_id"` | |
| ### `dataset` Examples | |
| ### [`MicroscopyDataset`](../reference/dreem/datasets/microscopy_dataset.md) | |
| * `videos`: (`list[str | list[str]]`) paths to raw microscopy videos | |
| * `tracks`: (`list[str]`) paths to trackmate gt labels (either `.xml` or `.csv`) | |
| * `source`: file format of gt labels based on label generator. Either `"trackmate"` or `"isbi"`. | |
| ### [`CellTrackingDataset`](../reference/dreem/datasets/cell_tracking_dataset.md) | |
| * `raw_images`: (`list[list[str] | list[list[str]]]`) paths to raw microscopy images | |
| * `gt_images`: (`list[list[str] | list[list[str]]]`) paths to gt label images | |
| * `gt_list`: (`list[str]`) An optional path to .txt file containing gt ids stored in cell tracking challenge format: `"track_id", "start_frame", "end_frame", "parent_id"` | |
| ### `dataset` Examples |
| Please see [here](configs/inference.md) for a walk through of the inference params as well as how to set up an inference conifg and see [here](configs/inference.md#example-config) for an example inference config file. | ||
| ### Step 3 Run Evaluation | ||
|
|
||
| Just like training we can use the hydra syntax for specifying arguments via the cli. Thus you can run inference via: |
There was a problem hiding this comment.
Fix the missing comma.
A comma may be missing after the conjunctive/linking adverb ‘Thus’.
- Thus you can run inference via:
+ Thus, you can run inference via: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Just like training we can use the hydra syntax for specifying arguments via the cli. Thus you can run inference via: | |
| Just like training we can use the hydra syntax for specifying arguments via the cli. Thus, you can run inference via: |
Tools
LanguageTool
[uncategorized] ~207-~207: Possible missing comma found.
Context: ...e. ### Step 3 Run Evaluation Just like training we can use the hydra syntax for specify...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~207-~207: A comma may be missing after the conjunctive/linking adverb ‘Thus’.
Context: ...x for specifying arguments via the cli. Thus you can run inference via: ```bash dre...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
| @@ -179,6 +179,70 @@ | |||
| ### Output | |||
| The output of the train script will be at least 1 `*.ckpt` file, assuming you've configured the `checkpointing` section of the config correctly and depending on the params you've used. | |||
|
|
|||
| ## Eval | |||
| Normally, testing will be done at the end of training assuming you passed in a test dataset. However, if you'd like to test your model on another dataset, we provide a way to do so here. Please [see above](./usage.md#step-1-generate-ground-truth-data) for how to set up your evaluation data. Basically we recommend storing your test videos/labels files as so to make things easier: | |||
There was a problem hiding this comment.
Fix the missing comma.
Consider adding a comma after ‘Basically’ for more clarity.
- Basically we recommend storing your test videos/labels files as so to make things easier:
+ Basically, we recommend storing your test videos/labels files as so to make things easier: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Normally, testing will be done at the end of training assuming you passed in a test dataset. However, if you'd like to test your model on another dataset, we provide a way to do so here. Please [see above](./usage.md#step-1-generate-ground-truth-data) for how to set up your evaluation data. Basically we recommend storing your test videos/labels files as so to make things easier: | |
| Normally, testing will be done at the end of training assuming you passed in a test dataset. However, if you'd like to test your model on another dataset, we provide a way to do so here. Please [see above](./usage.md#step-1-generate-ground-truth-data) for how to set up your evaluation data. Basically, we recommend storing your test videos/labels files as so to make things easier: |
Tools
LanguageTool
[uncategorized] ~183-~183: Possible missing comma found.
Context: ...lly, testing will be done at the end of training assuming you passed in a test dataset. ...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~183-~183: Consider adding a comma after ‘Basically’ for more clarity.
Context: ...for how to set up your evaluation data. Basically we recommend storing your test videos/l...(RB_LY_COMMA)
|
|
||
| ```bash | ||
| dreem-eval --config-base=[CONFIG_DIR] --config-name=[CONFIG_STEM] section.param=[VALUE] | ||
| ``` |
There was a problem hiding this comment.
Fix the abbreviation.
The abbreviation “e.g.” requires two periods.
- e.g if I want to set the window size of the tracker to 32 instead of 8 through `tracker.window_size=32` and use a different model saved in `/home/aaprasad/models/new_best.ckpt` I can do:
+ e.g. if I want to set the window size of the tracker to 32 instead of 8 through `tracker.window_size=32` and use a different model saved in `/home/aaprasad/models/new_best.ckpt` I can do: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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| e.g. if I want to set the window size of the tracker to 32 instead of 8 through `tracker.window_size=32` and use a different model saved in `/home/aaprasad/models/new_best.ckpt` I can do: |
Tools
LanguageTool
[uncategorized] ~226-~226: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...[CONFIG_STEM] section.param=[VALUE] ``` e.g if I want to set the window size of the...(E_G)
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
dreem/io/config.py (1)
423-438: Update function calls to match the new signatureThe calls to
get_trainerin the following files do not match the new signature defined indreem/io/config.pyand need to be updated to include the required parameters:
dreem/inference/track.pydreem/inference/eval.pyEnsure that these calls are updated to match the new function signature.
Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
get_trainermatch the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `get_trainer` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'get_trainer'Length of output: 66
Script:
#!/bin/bash # Description: Verify all function calls to `get_trainer` match the new signature. # Test: Search for the function usage in all Python files. Expect: Only occurrences of the new signature. rg 'get_trainer' --glob '*.py' -A 5Length of output: 1493
Script:
#!/bin/bash # Description: Extract and review the relevant lines from files where `get_trainer` is called. # Extract lines from dreem/training/train.py rg -A 5 'get_trainer' dreem/training/train.py # Extract lines from dreem/inference/track.py rg -A 5 'get_trainer' dreem/inference/track.py # Extract lines from dreem/inference/eval.py rg -A 5 'get_trainer' dreem/inference/eval.pyLength of output: 823
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- dreem/io/config.py (11 hunks)
Additional context used
Ruff
dreem/io/config.py
115-115: Undefined name
GTRRunner(F821)
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
Additional comments not posted (2)
dreem/io/config.py (2)
Line range hint
196-246:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
get_datasetmatch the new signature.Tools
Ruff
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
Line range hint
115-144:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
get_gtr_runnermatch the new signature.Verification successful
Function usage verified.
All calls to
get_gtr_runnermatch the new signature with the defaultckpt_path=None.
tests/test_config.py: gtr_runner = cfg.get_gtr_runner()dreem/training/train.py: model = train_cfg.get_gtr_runner()Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `get_gtr_runner` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'get_gtr_runner'Length of output: 69
Script:
#!/bin/bash # Description: Verify all function calls to `get_gtr_runner` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type py -A 5 $'get_gtr_runner'Length of output: 1031
Tools
Ruff
115-115: Undefined name
GTRRunner(F821)
| self, | ||
| mode: str, | ||
| label_files: list[str] | None = None, | ||
| vid_files: list[str | list[str]] = None, | ||
| ) -> "SleapDataset" | "MicroscopyDataset" | "CellTrackingDataset": |
There was a problem hiding this comment.
Fix undefined names for dataset classes.
Ensure that SleapDataset, MicroscopyDataset, and CellTrackingDataset are imported correctly to avoid runtime errors.
+ from dreem.datasets import SleapDataset, MicroscopyDataset, CellTrackingDatasetCommittable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ) -> "SleapDataset" | "MicroscopyDataset" | "CellTrackingDataset": | |
| + from dreem.datasets import SleapDataset, MicroscopyDataset, CellTrackingDataset | |
| ) -> "SleapDataset" | "MicroscopyDataset" | "CellTrackingDataset": |
Tools
Ruff
200-200: Undefined name
SleapDataset(F821)
200-200: Undefined name
MicroscopyDataset(F821)
200-200: Undefined name
CellTrackingDataset(F821)
| @@ -111,18 +112,27 @@ | |||
| tracker_cfg[key] = val | |||
| return tracker_cfg | |||
|
|
|||
| def get_gtr_runner(self) -> "GTRRunner": | |||
| """Get lightning module for training, validation, and inference.""" | |||
| def get_gtr_runner(self, ckpt_path=None) -> "GTRRunner": | |||
There was a problem hiding this comment.
Fix undefined name GTRRunner.
Ensure that GTRRunner is imported correctly to avoid runtime errors.
+ from dreem.models import GTRRunnerCommittable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_gtr_runner(self, ckpt_path=None) -> "GTRRunner": | |
| from dreem.models import GTRRunner | |
| def get_gtr_runner(self, ckpt_path=None) -> "GTRRunner": |
Tools
Ruff
115-115: Undefined name
GTRRunner(F821)
Here we add an eval script so you can run
dreem-evaland get out metrics on a test setSummary by CodeRabbit
New Features
dreem-trainfor training,dreem-evalfor evaluation, anddreem-trackfor inference.eval.yamlfor evaluation settings and streamlined evaluation processes witheval.py.track.pyscript with improved export functionality.Configclass for greater flexibility in dataset management.Documentation
README.mdandusage.mdto reflect new command-line instructions.docs/cli.mdfor detailed CLI command descriptions.docs/index.mdto display GitHub stars.Chores
logging.yamlconfiguration for structured logging management.