-
Notifications
You must be signed in to change notification settings - Fork 41
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
Suppress TensorFlow info messages to debug level #721
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #721 +/- ##
=======================================
Coverage 99.66% 99.67%
=======================================
Files 89 89
Lines 6356 6380 +24
=======================================
+ Hits 6335 6359 +24
Misses 21 21
☔ View full report in Codecov by Sentry. |
I tested this, but it didn't seem to do anything. After merging this PR branch to my local main branch, I'm seeing this:
This is exactly the same messages I'm seeing without this PR applied. |
Hmh, I had tested with just Now the TF log level is kept at 1 also after imports. However, the level can be controlled by setting the env in commandline:
|
This didn't help either. I'm still getting the same messages. I added a bit of debugging and it seems to me that the code you've inserted is not being run at all when I run the I also tried commenting out all the tensorflow/keras imports annif/backend/nn_ensemble.py - even that didn't help. It seems to me that tensorflow is being indirectly imported by something outside the annif codebase. A-ha! It seems that importing spaCy also imports TensorFlow if it has been installed:
|
Well this should work even for TensorFlow usage by spaCy. |
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.
Now it works even with spaCy!
One question though: would it make sense to set the TF logging level based on the Annif ( |
It would be nice, but then the question is how the levels should be aligned: the INFO messages of TF are the ones we want to hide, while at the same time show Annif INFO messages(?). One way for this would be to make TF log level "one more" than Annif log level (while Annif shows INFO+WARN+ERROR messages, TF shows WARN+ERROR messages; while Annif shows DEBUG+INFO+WARN+ERROR messages, TF shows INFO+WARN+ERROR). |
The "one more" idea sounds very good! I think it's reasonable to "tone down" log messages from TF by one level. They may sometimes be relevant for debugging problems, so making it possible (but not the default) to see messages from TF would be a nice feature. |
Now the feature works for regular commands: TensorFlow INFO messages are shown only when running Annif at DEBUG log level.
But I could not make it work in API server mode, I mean with |
The test for this needs to fixed... I'll try something with |
If this test is run after other tests with @mock.patch.dict(os.environ,...), the tests with caplog fixture in test_config.py fail
Tone down only TensorFlow INFO level: INFO messages by TFare shown only when running Annif at DEBUG level
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Makes TensorFlow log level to be set by Annif log level (
--verbosity/-v
option of CLI commands) usingTF_CPP_MIN_LOG_LEVEL
env variable.The INFO level of TensorFlow is mapped to be used only on DEBUG level of Annif to hide the distracting INFO messages by TensorFlow. Closes #720. Based on this SO answer.
The env setting is in the
annif/__init__.py
to make the log level change work also when TF is used by spaCy analyzer (which was not apparent at first), in addition to TF usage by NN ensemble backend.I am not seeing the second message mentioned in the issue:
I hope also this message arises during TF import, so this PR should fix that part of the issue too (Edit: no, it did not, but the current approach should take care it). Could @osma verify this?