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

NN Archive Structural Update #82

Merged
merged 11 commits into from
Mar 11, 2024
Merged

NN Archive Structural Update #82

merged 11 commits into from
Mar 11, 2024

Conversation

conorsim
Copy link
Collaborator

@conorsim conorsim commented Mar 6, 2024

This introduces the following changes to the NN archive structure from some iteration on what is needed for DepthAI:

  • We introduce a new HeadMetadata for YOLO instance segmentation - HeadMetadataInstanceSegmentationYOLO.
  • We drop the stages block and instead replace it with model since any multi-executable NN cases will instead be handled by Head.metadata. For example, HeadMetadataInstanceSegmentationYOLO specifies postprocessor_path for the postprocessing step which uses a second blob to perform a multiplication operation.
  • We drop the connections block since we no longer have stages.
  • We remove the head_id field in the outputs block and instead introduce a Head.outputs block which can be more descriptive about the parameters required for a DepthAI parser.
  • We remove the stride parameter for object detection heads.
  • We rework the structure of anchors for object detection heads to be a triply nested list. We assume it's always in the order of smallest to largest outputs.

conorsim added 2 commits March 5, 2024 15:08

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@tersekmatija
Copy link
Collaborator

Would postprocessor_path be null if we use ONNX or some other format where we don't need to have 2 "executables"?

Copy link
Contributor

@jkbmrz jkbmrz left a comment

Choose a reason for hiding this comment

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

LGTM

@type metadata: HeadMetadata
@ivar metadata: Parameters required by head to run postprocessing.
"""

head_id: str = Field(description="Unique head identifier.")
outputs: Union[List[str], Dict[str, Union[str, List]]] = Field(
description="A list of output names from the `outputs` block of the archive or a dictionary mapping DepthAI parser names needed for the head to output names. The referenced outputs will be used by the DepthAI parser."
Copy link
Contributor

@jkbmrz jkbmrz Mar 7, 2024

Choose a reason for hiding this comment

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

I assume only the outputs relevant to the head operations must be listed here? I'm wondering how to identify those from the perspective of automatic NN archive generation. We could try drawing them from cfg model architecture but this might not be so straight-forward as the NN archive outputs now became the outputs of the executable and might differ from names in cfg (as far as I know).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jkbmrz I was considering overriding outputs for each HeadMetadata or at least providing some validation on the expected format of outputs for a given head. However, we would then need to define outputs on the HeadMetadata level (instead of Head) or have one really long validtor in Head (which I don't think is a good idea). If we move outputs to HeadMetadata, then we only have HeadMetadata attributes and no attributes specific to Head.

Thus, it might make sense removing HeadMetadata entirely and just having all the head types inherit from Head. Then, we overrride outputs for each head, further defining its type and validating possible param names (in the outputs keys). Thoughts on that idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could remove the HeadMetadata class and have all the head types inherit from Head but I'm still not sure how we would figure out automatically what outputs belong to a specific head. Or does overriding outputs for each head means predefining?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Commit 2dcc2c3 implements the idea

Copy link

github-actions bot commented Mar 7, 2024

Test Results

43 tests   43 ✅  33s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit f95efac.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Mar 7, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
3716 1780 48% 0% 🟢

New Files

File Coverage Status
luxonis_ml/nn_archive/config_building_blocks/base_models/head_outputs.py 0% 🟢
TOTAL 0% 🟢

Modified Files

File Coverage Status
luxonis_ml/nn_archive/archive_generator.py 0% 🟢
luxonis_ml/nn_archive/config.py 0% 🟢
luxonis_ml/nn_archive/config_building_blocks/init.py 0% 🟢
luxonis_ml/nn_archive/config_building_blocks/base_models/init.py 0% 🟢
luxonis_ml/nn_archive/config_building_blocks/base_models/head.py 0% 🟢
luxonis_ml/nn_archive/config_building_blocks/base_models/output.py 0% 🟢
luxonis_ml/nn_archive/model.py 0% 🟢
TOTAL 0% 🟢

updated for commit: f95efac by action🐍

@conorsim conorsim added the enhancement New feature or request label Mar 7, 2024
@tersekmatija
Copy link
Collaborator

@conorsim Let's always store JSONs as nnarchive.json or config.json to keep it consistent as discussed offline?

conorsim and others added 2 commits March 7, 2024 20:13
@jkbmrz
Copy link
Contributor

jkbmrz commented Mar 8, 2024

^ a small commit adding return statement to ArchiveGenerator.make_archive() method.

@conorsim conorsim requested a review from jkbmrz March 8, 2024 18:08
@jkbmrz
Copy link
Contributor

jkbmrz commented Mar 11, 2024

^ added support for LZMA compression and setting .tar.xz as the default nn archive option

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.

LGTM

@conorsim conorsim changed the title Rework/nn archive updates NN Archive Structural Update Mar 11, 2024
@conorsim conorsim merged commit af161d8 into dev Mar 11, 2024
12 checks passed
@conorsim conorsim deleted the rework/nn-archive-updates branch March 11, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants