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

Actualized some reference .dot files #1355

Merged

Conversation

ljaljushkin
Copy link
Contributor

Changes

Updated .dot files which are used as reference graphs in tests

Reason for changes

Real changes in the graph are hard to review when files are not actual

Related tickets

#1323

Tests

  • test_compressed_graph
  • test_autoq_precision_init
  • test_hawq_precision_init
  • test_get_ip_graph_with_merged_operations

@github-actions github-actions bot added NNCF Common Pull request that updates NNCF Common NNCF PT Pull requests that updates NNCF PyTorch labels Nov 7, 2022
@vshampor
Copy link
Contributor

vshampor commented Nov 7, 2022

If the reference files are wrong, why are the tests passing?

@ljaljushkin
Copy link
Contributor Author

If the reference files are wrong, why are the tests passing?
References were valid before, but there just were in a obsolete format.

test_get_ip_graph_with_merged_operations just compares node names from loaded graph.

write_dot_graph(merged_ip_graph, str(path_to_dot_file))
load_graph = read_dot_graph(str(path_to_dot_file))
sanitized_loaded_keys = [key.replace('\\n', '\n') for key in load_graph.nodes.keys()]

For this test meta data of node is not important.

Tests for the graphs compares the loaded graph. So the difference in node and edge order is not important.

@vshampor
Copy link
Contributor

vshampor commented Nov 7, 2022

Maybe we could set up a special function, or a conditional path determined by a boolean argument in the functions, that would only dump the nodes and edges w/o metadata, and rewrite these impacted tests to use these functions instead? No sense in keeping data in reference files which is not checked for anyway.

@ljaljushkin ljaljushkin merged commit adf6a9c into openvinotoolkit:develop Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NNCF Common Pull request that updates NNCF Common NNCF PT Pull requests that updates NNCF PyTorch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants