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

Add dumping of hierarchical config on export #2868

Merged
merged 3 commits into from
Feb 7, 2024
Merged

Conversation

sovrasov
Copy link
Contributor

@sovrasov sovrasov commented Feb 1, 2024

Summary

This PR is blocked by two issues:

  • In CLI tests semantic segmentation is created with a wrong number of classes / dataset is invalid, therefore, self.model.label_info = lit_module.meta_info raises an error (the model is created with default number of classes at first, changing the number of classes is not implemented)
  • label_tree_edges is incorrect: it contains label (child node, group name) pairs, although it should contain (child node, parent node). MAPI can't work with wrong information.

How to test

Checklist

  • I have added unit tests to cover my changes.​
  • I have added integration tests to cover my changes.​
  • I have added e2e tests for validation.
  • I have added the description of my changes into CHANGELOG in my target branch (e.g., CHANGELOG in develop).​
  • I have updated the documentation in my target branch accordingly (e.g., documentation in develop).
  • I have linked related issues.

License

  • I submit my code changes under the same Apache License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below).
# Copyright (C) 2023 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

@sungmanc
Copy link
Contributor

sungmanc commented Feb 2, 2024

label_tree_edges is incorrect: it contains label (child node, group name) pairs, although it should contain (child node, parent node). MAPI can't work with wrong information.

This means that what I made for the label_tree_edges is incorrect ? If so, I can't find the solution for this incorrect information.

@harimkang
Copy link
Contributor

  • In CLI tests semantic segmentation is created with a wrong number of classes / dataset is invalid, therefore, self.model.label_info = lit_module.meta_info raises an error (the model is created with default number of classes at first, changing the number of classes is not implemented)

I'm not sure if I'm understanding this correctly.
I added the feature to update num_classes in the CLI (#2861).
What you mean is that in the semantic segmentation case, num_classes +1 if background is not included in the label_list is wrong?

@sovrasov
Copy link
Contributor Author

sovrasov commented Feb 2, 2024

label_tree_edges is incorrect: it contains label (child node, group name) pairs, although it should contain (child node, parent node). MAPI can't work with wrong information.

This means that what I made for the label_tree_edges is incorrect ? If so, I can't find the solution for this incorrect information.

In general yes, in OTX 1.x label schema could provide a parent label for each particular label, so I represented the hierarchical graph as a list of nodes (child label, parent label). To me it's not clear why we can't do the same thing in OTX 2.0, since we have the same datumaro dataset as an input.

@sovrasov
Copy link
Contributor Author

sovrasov commented Feb 2, 2024

  • In CLI tests semantic segmentation is created with a wrong number of classes / dataset is invalid, therefore, self.model.label_info = lit_module.meta_info raises an error (the model is created with default number of classes at first, changing the number of classes is not implemented)

I'm not sure if I'm understanding this correctly. I added the feature to update num_classes in the CLI (#2861). What you mean is that in the semantic segmentation case, num_classes +1 if background is not included in the label_list is wrong?

I've seen the PR, it works for "normal" datasets with > 1 classes (but auto-config for h-cls is still missing). I'll dm you to describe the particular case with sseg. Probably, we need a workaround

@sungmanc
Copy link
Contributor

sungmanc commented Feb 2, 2024

In general yes, in OTX 1.x label schema could provide a parent label for each particular label, so I represented the hierarchical graph as a list of nodes (child label, parent label). To me it's not clear why we can't do the same thing in OTX 2.0, since we have the same datumaro dataset as an input.

In this PR(#2804), I added the information about the label_tree_edges to the HLabelInfo that means the [child label, parent label] information. So, what you mean is this implementation was invalid ? If so, I need to modify this.

image

@sovrasov
Copy link
Contributor Author

sovrasov commented Feb 2, 2024

In general yes, in OTX 1.x label schema could provide a parent label for each particular label, so I represented the hierarchical graph as a list of nodes (child label, parent label). To me it's not clear why we can't do the same thing in OTX 2.0, since we have the same datumaro dataset as an input.

In this PR(#2804), I added the information about the label_tree_edges to the HLabelInfo that means the [child label, parent label] information. So, what you mean is this implementation was invalid ? If so, I need to modify this.

image

For instance, in this snipped, I see ['Rectangle', 'Object__Rigid Group'] pair. If this (child node, parent node) pair is correct, 'Object__Rigid Group' should be a tree node ~ a class -> it should be in label_to_idx dict -> I don't see this

@sovrasov
Copy link
Contributor Author

sovrasov commented Feb 2, 2024

I think for better understanding, we can load this dataset with OTX 1.6, and see the data generated. I'd be the simplest way

@sungmanc
Copy link
Contributor

sungmanc commented Feb 5, 2024

For instance, in this snipped, I see ['Rectangle', 'Object__Rigid Group'] pair. If this (child node, parent node) pair is correct, 'Object__Rigid Group' should be a tree node ~ a class -> it should be in label_to_idx dict -> I don't see this

Got it, thanks. I'll modify. Please check this PR: #2883

@sovrasov sovrasov merged commit e74a9f4 into v2 Feb 7, 2024
8 checks passed
@sovrasov sovrasov deleted the vs/v2_h_cls_export branch February 7, 2024 09:51
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