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

update statshandler logging message #6051

Merged
merged 10 commits into from
Feb 25, 2023
Merged

Conversation

wyli
Copy link
Contributor

@wyli wyli commented Feb 22, 2023

Description

The recommendation in StatsHandler of setting the global logging config may not work in some cases (e.g. google colab and #5960) https://docs.python.org/3/library/logging.html#logging.basicConfig

logging.basicConfig(stream=sys.stdout, level=logging.INFO)

in general we shouldn't recommend changing the root logger directly.

This PR changes the message to tune the module level loggers, updated example message will be:

.the effective log level of ignite.engine.engine.Engine higher than INFO, StatsHandler may not generate logs,
please use the following code before running the engine to enable it: 
                                                                                
import logging                                                              
logging.getLogger('ignite.engine.engine.Engine').setLevel(logging.INFO) 

the updated commands are tested in the colab example:
https://colab.research.google.com/drive/1boqy7ENpKrqaJoxFlbHIBnIODAs1Ih1T

Types of changes

  • 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.

wyli added 2 commits February 22, 2023 22:36
Signed-off-by: Wenqi Li <[email protected]>
Signed-off-by: Wenqi Li <[email protected]>
Copy link
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think the change is acceptable now. But please verify it with some bundles and workflow tutorials before merging.

Thanks.

@wyli
Copy link
Contributor Author

wyli commented Feb 25, 2023

OK, I think the change is acceptable now. But please verify it with some bundles and workflow tutorials before merging.

Thanks.

thanks, I've tested it in a bundle and tutorials, after merging this PR

this global setting can be avoided:

logging.basicConfig(stream=sys.stdout, level=logging.INFO)

new usage would be, either specify a logger name when creating StatsHandler:

monai.handlers.StatsHandler(name="train_stats"...)

or update the engine logger using ignite.utils.setup_logger:

trainer = ignite.engine.Engine(lambda x, y: [0.0])
ignite.utils.setup_logger(trainer.logger.name, reset=True)
monai.handlers.StatsHandler().attach(trainer)
trainer.run(range(3), max_epochs=4)

@wyli
Copy link
Contributor Author

wyli commented Feb 25, 2023

/build

@wyli wyli enabled auto-merge (squash) February 25, 2023 17:35
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Copy link
Contributor Author

wyli commented Feb 25, 2023

/build

@wyli wyli merged commit f519699 into Project-MONAI:dev Feb 25, 2023
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Feb 27, 2023

OK, I think the change is acceptable now. But please verify it with some bundles and workflow tutorials before merging.
Thanks.

thanks, I've tested it in a bundle and tutorials, after merging this PR

this global setting can be avoided:

logging.basicConfig(stream=sys.stdout, level=logging.INFO)

new usage would be, either specify a logger name when creating StatsHandler:

monai.handlers.StatsHandler(name="train_stats"...)

or update the engine logger using ignite.utils.setup_logger:

trainer = ignite.engine.Engine(lambda x, y: [0.0])
ignite.utils.setup_logger(trainer.logger.name, reset=True)
monai.handlers.StatsHandler().attach(trainer)
trainer.run(range(3), max_epochs=4)

Hi @wyli ,

I think your examples are python level, could you please share some usage examples for the bundle config?
Currently, bundles usually leverage this line config to run with correct logging:
https://github.com/Project-MONAI/model-zoo/blob/dev/models/spleen_ct_segmentation/configs/logging.conf#L11
Please show the recommendation change or how to update the existing bundles?
CC @yiheng-wang-nv .

Thanks in advance.

@wyli wyli deleted the suggest-logger branch February 27, 2023 10:36
mingxin-zheng pushed a commit to Project-MONAI/tutorials that referenced this pull request Feb 27, 2023
### Description
- Project-MONAI/MONAI#6034
- Project-MONAI/MONAI#6051

### Checks
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Avoid including large-size files in the PR.
- [x] Clean up long text outputs from code cells in the notebook.
- [x] For security purposes, please check the contents and remove any
sensitive info such as user names and private key.
- [x] Ensure (1) hyperlinks and markdown anchors are working (2) use
relative paths for tutorial repo files (3) put figure and graphs in the
`./figure` folder
- [x] Notebook runs automatically `./runner.sh -t <path to .ipynb file>`

---------

Signed-off-by: Wenqi Li <[email protected]>
boneseva pushed a commit to boneseva/MONAI-tutorials that referenced this pull request Apr 21, 2024
### Description
- Project-MONAI/MONAI#6034
- Project-MONAI/MONAI#6051

### Checks
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Avoid including large-size files in the PR.
- [x] Clean up long text outputs from code cells in the notebook.
- [x] For security purposes, please check the contents and remove any
sensitive info such as user names and private key.
- [x] Ensure (1) hyperlinks and markdown anchors are working (2) use
relative paths for tutorial repo files (3) put figure and graphs in the
`./figure` folder
- [x] Notebook runs automatically `./runner.sh -t <path to .ipynb file>`

---------

Signed-off-by: Wenqi Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants