Use the logger instead of stderr for warnings in the pbc modules - #2316
william-dawson wants to merge 2 commits into
Conversation
|
I think most of these warnings are important and are not meant to be suppressed. |
|
@fishjojo I agree, and I don't want to give the impression that I'm ignoring these warnings for production. With this modification, the warnings are still printed out by default. The change just has them be printed out in the same way as as equivalent warnings in other parts of the code. |
| sys.stderr.write('WARN: KNumInt.eval_ao function finds keyword ' | ||
| 'argument "kpt" and converts it to "kpts"\n') | ||
| logger.warn(SimpleNamespace(verbose=verbose), | ||
| 'KNumInt.eval_ao function finds keyword ' |
There was a problem hiding this comment.
SimpleNamespace(verbose=verbose) fails with NameError: name 'verbose' is not defined . Change this to cell.
For the critical issues, how about changing them to errors instead of just warnings? Wrapping other warnings should be okay IMO. |
okay. I think we can also add an option to always print, since sometimes we don't want the calculation to fail but let the program to choose the correct setting. |
`verbose` would be None by default, not an integer or logger type
| if 'kpt' in kwargs: | ||
| sys.stderr.write('WARN: KNumInt.nr_rks function finds keyword ' | ||
| 'argument "kpt" and converts it to "kpts"\n') | ||
| logger.warn(self, 'KNumInt.nr_rks function finds ' |
There was a problem hiding this comment.
Please raise an error for this incorrect kwarg.
| if 'kpt' in kwargs: | ||
| sys.stderr.write('WARN: KNumInt.nr_uks function finds keyword ' | ||
| 'argument "kpt" and converts it to "kpts"\n') | ||
| logger.warn(self, 'KNumInt.nr_uks function finds ' |
There was a problem hiding this comment.
Please raise an error for this incorrect kwarg.
|
Other sys.stderr can be replace IMO. @fishjojo please mark any critical warnings that you believe should not be suppressed. |
I agree. I don't have more comments. |
Inside the pbc modules, many times
sys.stderr.writeis used to write warnings, instead of thewarnmember function ofpyscf.lib.logger. This makes it difficult to suppress warnings. I modified each of those spots to instead call the logger.In all but one place, I could pass
selftowarnto ensure the verbosity parameter was communicated. However, inpyscf/pbc/dft/numint.py, the functioneval_aotakes instead an integer directly for the verbosity. This is why I use theSimpleNamespacetrick there, but if you prefer another approach let me know.