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

Extend NN Archive Generation Test Coverage #18

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

jkbmrz
Copy link
Contributor

@jkbmrz jkbmrz commented Mar 22, 2024

Extending test coverage of NN archive generation for each type of head in luxonis-train. For each head, we now have the following tests implemented:

  • Test if NN archive was created using xz compression (should be the default option)
  • Test if NN archive consists of config.json and model.onnx.
  • Test if archive ONNX model is valid.
  • Test if archived config inputs and outputs are valid.
  • Test if archived config head outputs are valid.

@jkbmrz jkbmrz requested a review from conorsim March 22, 2024 14:43
@jkbmrz jkbmrz changed the title Extending NN Archive Generation Test Coverage Extend NN Archive Generation Test Coverage Mar 22, 2024
Copy link

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
4658 3744 80% 0% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: b3ea9d7 by action🐍

@conorsim conorsim requested a review from kozlov721 March 22, 2024 15:29
Copy link
Collaborator

@conorsim conorsim left a comment

Choose a reason for hiding this comment

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

Thanks - LGTM

for i in range(10):
img = np.random.randint(0, 256, (10, 10, 3), dtype=np.uint8)
img_file_path = os.path.join(cls.tmp_path, "images", f"img{i}.png")
def _make_dummy_ldf(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not important and just an idea, but I could see this being useful across other tests too. We could make a LuxonisDatasetFactory as a shared factory which can be used in other tests as well.

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 also make sense to instead implement it in luxonis-ml and import it here since it would be useful for luxonis-ml tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, it makes sense to move it to luxonis-ml!

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

@jkbmrz jkbmrz merged commit b3b4e32 into dev Mar 25, 2024
@jkbmrz jkbmrz deleted the feature/archive-head-test-coverage branch March 25, 2024 09:12
kozlov721 pushed a commit that referenced this pull request Oct 9, 2024
* extend NN Archive generation test coverage to cover all implemented heads

* [Automated] Updated coverage badge

---------

Co-authored-by: GitHub Actions <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants