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

Enhance bundle logging logic #7513

Open
KumoLiu opened this issue Mar 4, 2024 · 4 comments · May be fixed by #7811
Open

Enhance bundle logging logic #7513

KumoLiu opened this issue Mar 4, 2024 · 4 comments · May be fixed by #7811

Comments

@KumoLiu
Copy link
Contributor

KumoLiu commented Mar 4, 2024

There are issues with using logging.conf to document the arguments recorded by _log_input_summary. Additionally, when more than one GPU is utilized, the output becomes messy and unmanageable.

https://github.com/Project-MONAI/model-zoo/blob/dev/models/brats_mri_generative_diffusion/configs/logging.conf

def _log_input_summary(tag: str, args: dict) -> None:

@hyacinth97223
Copy link

hyacinth97223 commented Apr 23, 2024

Hi, @KumoLiu
I am very interested in this issue, but I have the following questions for you:

  1. What does the problem you mentioned with using logging.conf to document the arguments recorded by _log_input_summary mean? Could you please describe it in more detail?
  2. Regarding the issue that output becomes messy and unmanageable, I have a method. I want to use buffer to record the GPU ID. Does this meet your needs?

Thanks.

hyacinth97223 pushed a commit to hyacinth97223/MONAI that referenced this issue May 27, 2024
Modify the function _log_input_summary() at 2024/05/27
Signed-off-by: Ted.Lai <hyacinth97223>
hyacinth97223 pushed a commit to hyacinth97223/MONAI that referenced this issue May 27, 2024
Modify the function _log_input_summary() at 2024/05/27
Signed-off-by: Ted.Lai <[email protected]>
hyacinth97223 pushed a commit to hyacinth97223/MONAI that referenced this issue May 27, 2024
Modify the function _log_input_summary() at 2024/05/27
Signed-off-by: Ted.Lai <[email protected]>
hyacinth97223 added a commit to hyacinth97223/MONAI that referenced this issue May 27, 2024
@hyacinth97223 hyacinth97223 linked a pull request May 27, 2024 that will close this issue
7 tasks
@KumoLiu
Copy link
Contributor Author

KumoLiu commented May 27, 2024

Hi @hyacinth97223, thanks for your interest here.

  1. What does the problem you mentioned with using logging.conf to document the arguments recorded by _log_input_summary mean? Could you please describe it in more detail?

For this one, I mean users can directly log the arguments to a local file using logging.conf.

  1. Regarding the issue that output becomes messy and unmanageable, I have a method. I want to use a log to record the GPU ID. Does this meet your needs?

For multi-gpu, there will be a lot of duplicate code that is not very well understood. Such as:

2024-05-27 12:31:45,604 - INFO - --- input summary of monai.bundle.scripts.run ---
2024-05-27 12:31:45,604 - INFO - --- input summary of monai.bundle.scripts.run ---
2024-05-27 12:31:45,604 - INFO - > config_file: ['configs/train.json', 'configs/multi_gpu_train.json']
2024-05-27 12:31:45,604 - INFO - ---
2024-05-27 12:31:45,604 - INFO - > config_file: ['configs/train.json', 'configs/multi_gpu_train.json']
2024-05-27 12:31:45,604 - INFO - ---
monai.bundle.workflows ConfigWorkflow.__init__:workflow_type: Current default value of argument `workflow_type=None` has been deprecated since version 1.2. It will be changed to `workflow_type=train` in version 1.4.
monai.bundle.workflows ConfigWorkflow.__init__:workflow_type: Current default value of argument `workflow_type=None` has been deprecated since version 1.2. It will be changed to `workflow_type=train` in version 1.4.
2024-05-27 12:31:45,604 - INFO - Setting logging properties based on config: configs/logging.conf.
2024-05-27 12:31:45,604 - INFO - Setting logging properties based on config: configs/logging.conf.
Loading dataset: 100%|██████████████████████████████████████████| 32/32 [00:41<00:00,  1.30s/it]
Loading dataset: 100%|██████████████████████████████████████████| 32/32 [00:41<00:00,  1.31s/it]
Loading dataset: 100%|███████████████| 9/9 [00:17<00:00,  1.93s/it]
Loading dataset: 100%|███████████████| 9/9 [00:17<00:00,  1.94s/it]

@hyacinth97223
Copy link

hyacinth97223 commented Jul 23, 2024

Hi, @KumoLiu
I have an idea and implemented it as follows. Would this meet your requirements and expectations?

I modified the _log_input_summary() function to include the following enhancements:

Single GPU (rank 0 only): If there is only one GPU, the function will exclusively output all arguments of rank 0 and create a log file to record these arguments.
Multiple GPUs: In a multi-GPU environment, the function will output the content of each thread separately, creating a distinct log file for each thread to record its respective arguments.

I wrote a multi-threaded program to test the functionality of this function, and the output is as shown in the image below.

output

The generated log files are as shown in the image below.

log
log_content

I would greatly appreciate your feedback on whether this implementation meets your expectations and requirements. Please let me know if you have any further questions or modifications.

@KumoLiu
Copy link
Contributor Author

KumoLiu commented Jul 23, 2024

Hi @hyacinth97223, thanks for your interest here again!
The expected behavior looks good to me.
For the first one, did you try to modify the logic which make users can directly set the logging path in the "logging.conf" ?
https://docs.python.org/3/library/logging.config.html#logging-config-fileformat
For the second one, I guess we can utilize the RankFilter.

class RankFilter(Filter):

BTW, I didn't see where you push the code?

KumoLiu added a commit to KumoLiu/MONAI that referenced this issue Oct 11, 2024
KumoLiu added a commit that referenced this issue Oct 23, 2024
… the bundle (#8142)

Fix Project-MONAI/model-zoo#658, part of #7513

### Description
Enable redirection of all loggers by configuring a FileHandler within
the bundle

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: YunLiu <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants