-
Notifications
You must be signed in to change notification settings - Fork 21
Issue #542 - folder structure rearrange - submission generation #574
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
Conversation
- user.conf and measurements.json to the scenario folder - system_meta.json to root folder
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
|
|
||
| user_conf_path = os.path.join(result_scenario_path, "user.conf") | ||
|
|
||
| measurements_json_path = os.path.join(result_scenario_path, "measurements.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to move this code to near line number 422 to avoid redundancy.
| shutil.copy(measurements_json_path, os.path.join(target_measurement_json_path, sub_res+'.json')) | ||
| shutil.copy(measurements_json_path, os.path.join(target_measurement_json_path, 'model-info.json')) | ||
| else: | ||
| if not mode.startswith("TEST"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should error out only in performance mode right?
| # This line can be removed once the PR in the inference repo is merged. | ||
| shutil.copy(measurements_json_path, os.path.join(submission_measurement_path, sub_res+'.json')) | ||
| shutil.copy(measurements_json_path, os.path.join(submission_measurement_path, 'model-info.json')) | ||
| shutil.copy(measurements_json_path, os.path.join(target_measurement_json_path, sub_res+'.json')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR is merged. So, we can remove this line
| if not all([os.path.exists(os.path.join(result_scenario_path, "performance", "run_1", f)) for f in files_to_check]): | ||
| continue | ||
|
|
||
| if not os.path.isdir(measurement_scenario_path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @arjunsuresh , i have placed it above the for loop as submission_measurement_path and measurement_scenario_path are the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay 👍
| tags: _short-run | ||
| env: | ||
| CM_MLPERF_SUBMISSION_DIVISION: open | ||
| CM_RUN_MLPERF_SUBMISSION_PREPROCESSOR: off |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theres already an env in this variation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missed due to space in between... fixed:)
No description provided.