-
Notifications
You must be signed in to change notification settings - Fork 70
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
Holoscan Flow Benchmarking Logging Updates #555
Holoscan Flow Benchmarking Logging Updates #555
Conversation
466e7ff
to
157d8ee
Compare
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.
@tbirdso Great improvements! I have left a few suggestions which are necessary, in my opinion.
Also, could you kindly show some sample outputs of the logging
module in python that we are getting it here.
Changes: - Adopts standard `logging` module to enhance granular control and redirection of benchmark script logging - Updates missing log files from warning to error severity Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
Refactors code and addresses issue where log file parent directory was not appended to path search for existence, resulting in only the local directory being checked for log files existence. NVBug 4933187 Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
157d8ee
to
89dfefd
Compare
Changes: - Add `--level` option to control logging verbosity and use `logger` object throughout - Refactor logging so that each output filepath is printed exactly once. Available files are printed with INFO verbosity, missing files are printed with ERROR verbosity. - Exit with error if log files are missing Sample log printout: ``` 2024-11-08 05:23:47,323 INFO: Log file directory: /workspace/holohub/log_directory_20241108-052346 2024-11-08 05:23:47,324 INFO: Log files are available: 2024-11-08 05:23:47,324 ERROR: Log files are missing: /workspace/holohub/log_directory_20241108-052346/logger_greedy_1_1.log 2024-11-08 05:23:47,324 ERROR: Some log files are missing. Please check the log directory. ``` Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
Updates `run` script to propagate errors from `run launch` as the `run` script exit code. Allows Holoscan Flow Benchmarking tools to detect when a HoloHub application process exits abnormally. Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
Changes: - Adds logging handler to log detailed (DEBUG) output to a "benchmark.log" file in the given run log directory. Adds a stream handler to preserve control over CLI output verbosity with the "--level" CLI argument. - Logs stdout and stderr from an failing process to aid error investigation. Includes thread ID. Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
89dfefd
to
06e815f
Compare
Added quality-of-life updates:
Failed run example (headless SSH):
Corresponding Successful run example, with an additional run and GPU utilization enabled for purpose of demonstration:
Corresponding Failed run with console logging limited to errors only:
These updates aim to improve the |
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.
LGTM
Changes:
benchmark.py
to use standard Python logging moduleLog files may be missing if the underlying
./run launch
command fails to run the application, such as in the event of a bad app configuration or run environment.