Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/inference #82

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Feat/inference #82

merged 3 commits into from
Sep 26, 2024

Conversation

JSabadin
Copy link
Contributor

@JSabadin JSabadin commented Sep 26, 2024

Inference Functionality Updates

  • Refactored the infer() method with the following helper methods:

    • _process_single_image: Handles inference for a single image file.
    • _process_directory_images: Handles inference for image files in a directory.
    • _process_dataset_images: Handles inference on dataset images.
    • _prepare_labels: Prepares labels for various tasks.
  • The infer() method now supports:

    • Single Image File inference.
    • Directory of Images inference.
    • Luxonis Dataset inference

@JSabadin JSabadin self-assigned this Sep 26, 2024
@github-actions github-actions bot added enhancement New feature or request tests Adding or changing tests DevOps Changes related to DevOps CLI Changes affecting the CLI release New version release labels Sep 26, 2024
@JSabadin JSabadin requested review from klemen1999, kozlov721 and tersekmatija and removed request for tersekmatija September 26, 2024 08:38
@kozlov721 kozlov721 changed the base branch from main to dev September 26, 2024 08:40
@kozlov721 kozlov721 requested a review from a team as a code owner September 26, 2024 08:40
@kozlov721 kozlov721 requested review from tersekmatija and conorsim and removed request for a team September 26, 2024 08:40
Copy link
Collaborator

@kozlov721 kozlov721 left a comment

Choose a reason for hiding this comment

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

Left a few nit-picky comments, otherwise LGTM! (once the checks pass)

@@ -17,7 +18,7 @@
from luxonis_ml.nn_archive.config import CONFIG_VERSION
from luxonis_ml.utils import LuxonisFileSystem, reset_logging, setup_logging
from typeguard import typechecked

from luxonis_ml.data import LabelType
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use luxonis_train.enums.TaskType instead. (see #78)

@@ -419,6 +420,7 @@ def infer(
self,
view: Literal["train", "val", "test"] = "val",
save_dir: str | Path | None = None,
img_path: Optional[str] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of Optional[str], you should use str | None (Optional is deprecated in python 3.10)

else:
self._process_dataset_images(view, save_dir)

def _process_single_image(self, img_path: Path, view: str, save_dir: Optional[str | Path]) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be better to move these methods to luxonis_train.core.utils.infer_utils

@@ -50,6 +50,11 @@ class _ViewType(str, Enum):
typer.Option(help="Where to save the inference results."),
]

ImgPathType = Annotated[
Optional[str],
Copy link
Collaborator

Choose a reason for hiding this comment

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

str | None

for inputs, labels in self.pytorch_loaders[view]:
images = get_unnormalized_images(self.cfg, inputs)
outputs = self.lightning_module.forward(
inputs, labels, images=images, compute_visualizations=True
)
render_visualizations(outputs.visualizations, save_dir)

def _prepare_labels(self, view: str, img_shape: tuple) -> tuple:
Copy link
Collaborator

Choose a reason for hiding this comment

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

_create_dummy_labels might be a better name

Copy link
Collaborator

@klemen1999 klemen1999 left a comment

Choose a reason for hiding this comment

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

LGTM, left some comments

luxonis_train/core/utils/infer_utils.py Outdated Show resolved Hide resolved
luxonis_train/core/core.py Outdated Show resolved Hide resolved
@JSabadin JSabadin requested a review from klemen1999 September 26, 2024 13:25
Copy link
Collaborator

@klemen1999 klemen1999 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

Attention: Patch coverage is 30.95238% with 29 lines in your changes missing coverage. Please review.

Please upload report for BASE (dev@be983ba). Learn more about missing BASE report.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
luxonis_train/core/utils/infer_utils.py 30.30% 23 Missing ⚠️
luxonis_train/core/core.py 33.33% 6 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##             dev      #82   +/-   ##
======================================
  Coverage       ?   96.21%           
======================================
  Files          ?      139           
  Lines          ?     6148           
  Branches       ?        0           
======================================
  Hits           ?     5915           
  Misses         ?      233           
  Partials       ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JSabadin JSabadin merged commit 706d3d0 into dev Sep 26, 2024
9 checks passed
@JSabadin JSabadin deleted the feat/inference branch September 26, 2024 14:20
kozlov721 pushed a commit that referenced this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Changes affecting the CLI DevOps Changes related to DevOps enhancement New feature or request release New version release tests Adding or changing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants