-
Notifications
You must be signed in to change notification settings - Fork 0
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
Log summary of simulation statuses #46
Conversation
Minimum allowed coverage is Generated by 🐒 cobertura-action against da029d7 |
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.
Thanks Natalie! This looks great and would be really helpful! I just added some thought to get your feedback on those and see if implementing those would add something to our guardrails. Thanks!
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.
Thanks, Natalie!
for upgrade in postprocessing.get_upgrade_list(self.cfg): | ||
if f"{upgrade:02d}" not in status_summary: | ||
s += f"\nNo results found for Upgrade {upgrade}" | ||
logger.info(s) |
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.
What do you think of also including:
-
The sum for each status (e.g., total successes and total failures)
-
The total of all statuses counted, and a warning (of sorts) if it's less than the total number of sims?
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.
Sums: Sure, I can add that.
Comparing to the total: This is possible, but kind of annoying to do at this point in the job. To find the total number of simulations that should exist, I think we have to pull buildstock.csv back out of assets.tar.gz on the cloud and re-count how many rows it has. (We already have this info locally if running the full pipeline, but may not if running postprocessing only.) Thoughts on how important this is? Or a simpler way to find the intended total?
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.
If the total isn't readily available, I think it's fine to leave it out. We can easily pull that from elsewhere if needed.
At the end of post-processing, print a summary of successful/failed/invalid simulations, grouped by upgrade.
Example output:
Future work: