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

Seed checker gets confused when handling multiple implementations #129

Open
emizan76 opened this issue May 19, 2021 · 8 comments
Open

Seed checker gets confused when handling multiple implementations #129

emizan76 opened this issue May 19, 2021 · 8 comments
Assignees
Labels

Comments

@emizan76
Copy link
Contributor

There is a bug in the seed checker:

On this line we see that

# Find all source codes for this benchmark.

the seed checker checks all source directories under implementation. So if we have two implementations, one in TF, and one in PyT for the same benchmark, then the following scenario is possible:

Assume the TF implementation reports the seed and the PyT does not. Then for the logs of the TF implementation the PyT source code will also be checked and will give a warning. This is rather confusing.

The solution is to have ONLY one implementation to be checked as each log file comes only from one implementation.

@shangw-nvidia
Copy link
Contributor

Hmmm... If one submission has two implementations, how do we know which implementation is the "real" implementation? And why would the "unused" implementation submitted in the first place?

@shangw-nvidia
Copy link
Contributor

shangw-nvidia commented May 19, 2021

Maybe the title should be change to "[Seed Checker] Unable to handle multiple implementation." "Bug on seed checker" is not very informative.

@emizan76 emizan76 changed the title Bug on seed checker Seed checker gets confused when handling multiple implementations May 19, 2021
@emizan76
Copy link
Contributor Author

The results dirs should have separate directories for these implementations.
For example here NVDA has 2 implementations for SSD:
https://github.com/mlcommons/training_results_v0.7/tree/master/NVIDIA/benchmarks/ssd/implementations

and there are 9 sets of results both mxnet and pytorch. For every mxnet log the pytorch source code is checked too and vice versa.

@shangw-nvidia
Copy link
Contributor

shangw-nvidia commented May 19, 2021

Do you know if it is a rule that says both the results dir and the implementation dir end with a "-{framework}" suffix? If so, maybe I can perform some string matching on the dir path; otherwise, solving this might be tricky.

@shangw-nvidia
Copy link
Contributor

https://github.com/mlcommons/training_results_v0.7/tree/master/Fujitsu/benchmarks/resnet/implementations/implementation_closed

is an example that I'm worried about. There's no way to figure out what implementation it is by looking at the path alone.

@emizan76
Copy link
Contributor Author

Good question. In the policies doc it says: "System names and implementation names may be arbitrary." https://github.com/mlcommons/policies/blob/master/submission_rules.adoc#561-training

Given that it is indeed hard to fix. I guess we need to keep that in mind when reviewing the submissions. So far I have hit just a warning, but I am not sure if there is another case where the seed checker fails while it should not.

@emizan76
Copy link
Contributor Author

This can be fixed, but only if we ensure a deterministic behavior between results directory and source code directory.

For example:
Result dir: https://github.com/mlcommons/training_results_v1.0/tree/master/Google/results/tpu-v4-128-TF/resnet

matches to implementation dir:
https://github.com/mlcommons/training_results_v1.0/tree/master/Google/benchmarks/resnet/implementations/resnet-preview-TF-tpu-v4-128

This is easy for a human to understand, but not deterministic for a script because the submission rules however it says that implementation dir names may be arbitrary (also there are 3 more implementation dirs referring to different results dirs)
https://github.com/mlcommons/policies/blob/master/submission_rules.adoc#:~:text=txt%20%23%20log%20file-,System%20names%20and%20implementation%20names%20may%20be%20arbitrary.,-Training%20benchmark%20directory

So this has to change -- implementation directory names should be well-defined to deterministically match results dirs.
Also this has to change if we want to match source code lines and log-reported lines automatically, see #74

What we do right now is check ALL implementation dirs for every results dir -- which is clearly wrong.

@emizan76
Copy link
Contributor Author

This depends on issue #156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants