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

Use logging for warnings always, not just sometimes? #4812

Open
ejfdickinson opened this issue Feb 1, 2025 · 4 comments
Open

Use logging for warnings always, not just sometimes? #4812

ejfdickinson opened this issue Feb 1, 2025 · 4 comments

Comments

@ejfdickinson
Copy link

ejfdickinson commented Feb 1, 2025

PyBaMM sometimes raises warnings to its own logger using the logging package, via pybamm.logger.warning(...).

PyBaMM also sometimes raises warnings using the native warn() functionality of the warnings package.

Sometimes these are even in the same method - pybamm.ParameterValues._create_from_bpx() does both.

Is there any reason why some warnings are logged, and some are not? I think it would be better to do one or the other, especially from the point-of-view of suppressing or explicitly handling warnings when interacting with PyBaMM from the outside.

@rtimms
Copy link
Contributor

rtimms commented Feb 2, 2025

Yeah we should probably try to be consistent. I’m not sure what best practice is here @martinjrobins @kratman ?

FYI you can catch logger warnings, see https://docs.python.org/3/library/logging.html#logging.captureWarnings

@agriyakhetarpal
Copy link
Member

The answers here are quite good: https://stackoverflow.com/q/9595009

I think one way could be to use the logger warnings for solver-specific cases (which can be set to debug/info/warn/etc. by a user) via set_logging_level() at the top of their scripts, and the rest of the from warnings import warn cases could stay as-is.

@ejfdickinson
Copy link
Author

Based on the existence of a canonical answer that explains the reasons to do one or the other (https://docs.python.org/3/howto/logging.html as linked from the post raised by @agriyakhetarpal above), I think the answer to my OP is probably "no". But there could be a case for reviewing specific instances. It's admittedly relatively easy to deal with both types of warnings, once one knows that the library might generate them.

@agriyakhetarpal
Copy link
Member

But there could be a case for reviewing specific instances

Yes, I agree, @ejfdickinson.

It's admittedly relatively easy to deal with both types of warnings, once one knows that the library might generate them.

One thing could be that we set up two separate loggers based on PyBaMM's modules, so that pybamm.solvers has its own logger whose logging level or output suppression can be customised by users, and the rest of the modules can have their logger.

Another way could be to customise the singular logger to reflect what is being output from where better: so that [INFO] base_model._build_model(550): Start building Doyle-Fuller-Newman model could point to the qualname of the class where the logger was first used to send out a message (I haven't explored how to do this but it should be possible in theory).

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

No branches or pull requests

3 participants