Suppression of warnings and (optionally) output #106

gibsramen · 2020-01-17T22:43:37Z

Addresses #105

Some users have mentioned that it would be useful to suppress the output of songbird.multinomial, including the TensorFlow deprecation warnings and tqdm progress bar. This PR should address most of these issues.

make test passes on my build (added a couple tests of the new functionality).

Main changes

Filter out some of the more obnoxious warnings when importing TensorFlow and biom.
Add silent flag to songbird.multinomial, allowing users the option of suppressing both the TensorFlow logging warnings and the tqdm progress bar.

See this SO question and this GitHub issue. Note that the latter apparently works only on TensorFlow 1.14.0. If/when this requirement is updated in Songbird, we may need to come back to this issue.

Python Warnings

Python FutureWarnings are now all filtered regardless of silent flag.

RuntimeWarnings also suppressed in standalone CLI.

$ songbird multinomial \
	--input-biom data/redsea/redsea.biom \
	--metadata-file data/redsea/redsea_metadata.txt \
	--formula "Depth+Temperature+Salinity+Oxygen+Fluorescence+Nitrate" \
	--epochs 10000 \
	--differential-prior 0.5 \
	--summary-interval 1 \
	--summary-dir results

Old behavior

/miniconda3/envs/evans_fxr/lib/python3.6/site-packages/tensorflow/python/framework/dtypes.py:516: FutureWarning: Passing (type, 1) or '1type' as a synonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
  _np_qint8 = np.dtype([("qint8", np.int8, 1)])
/miniconda3/envs/evans_fxr/lib/python3.6/site-packages/tensorflow/python/framework/dtypes.py:517: FutureWarning: Passing (type, 1) or '1type' as a synonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
.
.
.
/miniconda3/envs/evans_fxr/lib/python3.6/site-packages/biom/table.py:4049: FutureWarning: SparseSeries is deprecated and will be removed in a future version.
Use a Series with sparse values instead.

    >>> series = pd.Series(pd.SparseArray(...))

See http://pandas.pydata.org/pandas-docs/stable/user_guide/sparse.html#migrating for more.

  for r in self.matrix_data.tocsr()]
/miniconda3/envs/evans_fxr/lib/python3.6/site-packages/biom/table.py:4052: FutureWarning: SparseDataFrame is deprecated and will be removed in a future version.
Use a regular DataFrame whose columns are SparseArrays instead.
.
.
.
WARNING:tensorflow:From /miniconda3/envs/evans_fxr/bin/songbird:173: The name tf.Session is deprecated. Please use tf.compat.v1.Session instead.

2020-01-17 14:06:30.183343: I tensorflow/core/platform/cpu_feature_guard.cc:142] Your CPU supports instructions that this TensorFlow binary was not compiled to use: AVX2 FMA
WARNING:tensorflow:From /miniconda3/envs/evans_fxr/bin/songbird:176: The name tf.set_random_seed is deprecated. Please use tf.compat.v1.set_random_seed instead.
.
.
.
WARNING:tensorflow:From /miniconda3/envs/evans_fxr/lib/python3.6/site-packages/songbird/multinomial.py:163: The name tf.train.Saver is deprecated. Please use tf.compat.v1.train.Saver instead.

  0%|                                                            | 0/80000 [00:00<?, ?it/s]

New behavior

WARNING:tensorflow:From /Users/gibs/projects/songbird/scripts/songbird:199: The name tf.Session is deprecated. Please use tf.compat.v1.Session instead.

2020-01-17 14:35:40.991584: I tensorflow/core/platform/cpu_feature_guard.cc:142] Your CPU supports instructions that this TensorFlow binary was not compiled to use: AVX2 FMA
WARNING:tensorflow:From /Users/gibs/projects/songbird/scripts/songbird:202: The name tf.set_random_seed is deprecated. Please use tf.compat.v1.set_random_seed instead.

WARNING:tensorflow:From /Users/gibs/projects/songbird/songbird/multinomial.py:75: multinomial (from tensorflow.python.ops.random_ops) is deprecated and will be removed in a future version.
Instructions for updating:
Use `tf.random.categorical` instead.
WARNING:tensorflow:From /Users/gibs/projects/songbird/songbird/multinomial.py:86: The name tf.random_normal is deprecated. Please use tf.random.normal instead.
.
.
.
WARNING:tensorflow:From /Users/gibs/projects/songbird/songbird/multinomial.py:167: The name tf.train.Saver is deprecated. Please use tf.compat.v1.train.Saver instead.

  0%|                                                                | 0/80000 [00:00<?, ?it/s]

Silent flag

Both standalone and Q2 implementations can now specify whether or not to suppress output.

$ songbird multinomial \
	--input-biom data/redsea/redsea.biom \
	--metadata-file data/redsea/redsea_metadata.txt \
	--formula "Depth+Temperature+Salinity+Oxygen+Fluorescence+Nitrate" \
	--epochs 10000 \
	--differential-prior 0.5 \
	--summary-interval 1 \
	--summary-dir results \
        --silent; \
        echo "Finished"
Finished

NOTE: To my knowledge, calling qiime songbird multinomial from the CLI defaults to not showing the tqdm progress bar. In this case, the quiet flag is redundant and provided for compatibility. Calling through the Artifact API, however, outputs the tqdm progress bar normally, so the flag can be used there.

Turns out that you will see the progress bar from the Qiime2 invocation if you provide the --verbose option.

Importing tensorflow natively would throw FutureWarnings whenever the module was loaded. Now temporarily ignore FutureWarnings when importing tensorflow.

Now user can specify "quiet=True" to suppress tqdm output.

When quiet specified, multinomial will not print deprecation warnings or profiling messages.

Runtime and FutureWarnings now ignored when using songbird from the command line.

More efficient to filter warnings at the beginning rather than each time tensorflow is imported.

Can now specify --quiet or --no-quiet for CLI implementation. Also adds test that these commands work as expected.

Added tests that capture multinomial output to stderr to check that quiet does not return anything and that not-quiet does.

Default behavior description removed from quiet. Plugin setup also mentions that the quiet flag is only relevant for the Artifact API. My understanding is that the default behavior of q2-songbird from the CLI will *not* output the tqdm but the Artifact API does.

mortonjt

This is great. From a first pass, this is good to go once the flake8 issues are fixed.

mortonjt · 2020-01-18T04:46:51Z

songbird/q2/tests/test_method.py

+        md.name = 'sampleid'
+        md = qiime2.Metadata(md)
+
+        exp_beta = clr(clr_inv(np.hstack((np.zeros((2, 1)), self.beta.T))))


oof - we still have that code in there? That is definitely going to cause numerical issues

We should be instead doing

exp_beta = np.hstack((np.zeros((2, 1)), self.beta.T)) exp_beta = exp_beta - exp_beta.mean(axis=1).reshape(-1, 1)

In that way, we don't have to ever leave log space :)

If there are other instances of this, we need to fix them (in another PR)>

Looks like the only place clr() is being used at all in Songbird's codebase is in that line of testing code, so it's likely not a huge deal. (clr_inv() is being used in one other place, here (since it's getting imported there as softmax), but I don't know enough to know if that's a problem or not.)

wait jk it's also being used in scripts/songbird here

mortonjt · 2020-01-18T04:47:24Z

songbird/multinomial.py

+        if quiet:
+            iter_range = range(0, num_iter)
+        else:
+            iter_range = tqdm(range(0, num_iter))


whoa, interesting.

1) Removed unused exp_beta from quiet tests 2) Moved warning capture in standalone to avoid top-level import problems

gibsramen · 2020-01-21T17:29:14Z

Hmmm... I addressed the flake8 issues but now it looks like there's some sort of GLIBC error in the travis check. Is this something my changes brought about? (Not super familiar with C stuff)

fedarko

Looks great! I have a few small requests, mostly minor stuff for making life easier for users. If you have any questions just let me know.

fedarko · 2020-02-04T22:20:33Z

scripts/songbird

+    from tensorboard.plugins.hparams import api as hp
+    import tensorflow as tf
+
+warnings.filterwarnings("ignore", category=RuntimeWarning)


I don't really have any feedback here beyond "I didn't know this was a thing you can do in python and this is cool"

fedarko · 2020-02-04T22:21:49Z

songbird/parameter_info.py

@@ -36,6 +36,10 @@
        "for it to be included in the analysis."
    ),
    "summary-interval": "Number of seconds before storing a summary.",
+    "quiet": (
+        'Flag denoting whether to hide tqdm progress bar. If True, tqdm '
+        'bar is displayed. If False, tqdm is not printed.'


Might be worth just saying "progress bar" instead of "tqdm progress bar" or "tqdm bar", since I imagine many users might not know what tqdm is. But that's minor -- up to you whether or not you'd want to change it.

Oh, also -- it looks like quiet is also being used to hide TensorFlow warnings. That's fine, but the description should be updated accordingly (I doubt most people will care but some people might want to see these, at least the first time).

fedarko · 2020-02-04T22:38:01Z

scripts/songbird

+
+        # suppress deprecation warnings
+        # taken from https://github.com/tensorflow/tensorflow/issues/27023
+        tf.compat.v1.logging.set_verbosity(tf.compat.v1.logging.ERROR)


Just leaving a note tagging #110 -- for future reference, this will be one of the things that'll need to be changed upon updating to TensorFlow v2. (Should be ok to leave this in for now.)

fedarko · 2020-02-04T22:39:40Z

scripts/test_songbird_cli.py

+
+        result = runner.invoke(songbird.multinomial, test_args)
+
+        assert result.output != ""


These tests are super easy to understand -- this looks great.

fedarko · 2020-02-04T22:42:06Z

songbird/q2/_method.py

+
+        # suppress deprecation warnings
+        # taken from https://github.com/tensorflow/tensorflow/issues/27023
+        tf.compat.v1.logging.set_verbosity(tf.compat.v1.logging.ERROR)


Since it looks like this code is duplicated between here and in scripts/songbird, would it be possible to spin this off into a function in songbird/util.py and call that function from both locations? This would minimize maintenance burden (and we already know that the tf.compat.v1. thing will probably need to be rewritten at some point down the line...)

Yeah, this occurred to me as well. Was hesitant to create a new module without running it by anyone but if it's okay with you I'll go ahead and do it.

I don't think you'd need to create a new module -- it should be possible to just add a function in util.py that does this stuff, right?

🤦‍♂ yep...

fedarko · 2020-02-04T22:44:49Z

songbird/q2/plugin_setup.py

@@ -81,6 +82,7 @@
        "min_feature_count": DESCS["min-feature-count"],
        "summary_interval": DESCS["summary-interval"],
        "random_seed": DESCS["random-seed"],
+        "quiet": DESCS["quiet"] + " (Only affects Artifact API)",


I don't think this is correct -- you'll still see the progress bar if you include --verbose in the qiime songbird multinomial invocation. Would suggest revising to (Only has an impact when using this command with the --verbose option or through the QIIME 2 Artifact API), or something along those lines

For reference - just tried out the Qiime2 command with the --verbose flag (with --p-no-quiet) and and I can confirm it does display the progress bar & warnings. Will update documentation accordingly. 😄

songbird/q2/tests/test_method.py

songbird/parameter_info.py

Made more clear the effect the silent flag has and flipped the erroneously documentation of True/False behavior. Updated Qiime2 description of flag.

fedarko

Thanks @gibsramen! Everything looks good to me.

fedarko · 2020-02-05T23:39:35Z

@gibsramen I updated the changelog about this PR -- let me know if this description looks good/bad to you.

gibsramen added 10 commits January 16, 2020 15:38

Added FutureWarning capture to TF imports

ca17ae1

Importing tensorflow natively would throw FutureWarnings whenever the module was loaded. Now temporarily ignore FutureWarnings when importing tensorflow.

Added quiet flag to multinomial fit

96f985f

Now user can specify "quiet=True" to suppress tqdm output.

Added suppression in multinomial

bf4beb5

When quiet specified, multinomial will not print deprecation warnings or profiling messages.

Added warning suppression in CLI songbird

095fb3b

Runtime and FutureWarnings now ignored when using songbird from the command line.

Moved warning filter to __init__.py

141091b

More efficient to filter warnings at the beginning rather than each time tensorflow is imported.

Added quiet option for CLI songbird

551c4c0

Can now specify --quiet or --no-quiet for CLI implementation. Also adds test that these commands work as expected.

Added q2 tests for quiet output

1aed7cb

Added tests that capture multinomial output to stderr to check that quiet does not return anything and that not-quiet does.

Removed redundant warning captures in tests

b80ec12

Added links to code used for warning suppression

20ad254

mortonjt reviewed Jan 18, 2020

View reviewed changes

Address flake8 issues

f652c9d

1) Removed unused exp_beta from quiet tests 2) Moved warning capture in standalone to avoid top-level import problems

gibsramen mentioned this pull request Jan 22, 2020

Travis tests failing all of a sudden #107

Closed

gwarmstrong mentioned this pull request Jan 24, 2020

ENH: Q2 summary interval allow float #108

Merged

fedarko self-requested a review February 4, 2020 10:34

fedarko mentioned this pull request Feb 4, 2020

numerical issues #31

Open

fedarko requested changes Feb 4, 2020

View reviewed changes

gibsramen added 4 commits February 5, 2020 10:08

Changed quiet to silent

2306588

Update silent documentation

5d7e8fb

Made more clear the effect the silent flag has and flipped the erroneously documentation of True/False behavior. Updated Qiime2 description of flag.

Moved warning suppression to util

4138428

Abstracted out method silence tests

c478fa9

gibsramen requested a review from fedarko February 5, 2020 19:13

fedarko approved these changes Feb 5, 2020

View reviewed changes

fedarko merged commit af4801a into biocore:master Feb 5, 2020

fedarko mentioned this pull request Feb 5, 2020

Suppress all output while running multinomial #105

Closed

fedarko added a commit that referenced this pull request Feb 5, 2020

REL: update CHANGELOG re: #106

8c9011d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suppression of warnings and (optionally) output #106

Suppression of warnings and (optionally) output #106

gibsramen commented Jan 17, 2020 •

edited

Loading

mortonjt left a comment

mortonjt Jan 18, 2020

fedarko Feb 4, 2020

fedarko Feb 11, 2020

mortonjt Jan 18, 2020

gibsramen commented Jan 21, 2020

fedarko left a comment

fedarko Feb 4, 2020

fedarko Feb 4, 2020

fedarko Feb 4, 2020

fedarko Feb 4, 2020

fedarko Feb 4, 2020

fedarko Feb 4, 2020

gibsramen Feb 4, 2020

fedarko Feb 4, 2020

gibsramen Feb 4, 2020

fedarko Feb 4, 2020

gibsramen Feb 4, 2020

fedarko left a comment

fedarko commented Feb 5, 2020 •

edited

Loading


		result = runner.invoke(songbird.multinomial, test_args)

		assert result.output != ""

Suppression of warnings and (optionally) output #106

Suppression of warnings and (optionally) output #106

Conversation

gibsramen commented Jan 17, 2020 • edited Loading

Main changes

Python Warnings

Old behavior

New behavior

Silent flag

mortonjt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibsramen commented Jan 21, 2020

fedarko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedarko left a comment

Choose a reason for hiding this comment

fedarko commented Feb 5, 2020 • edited Loading

gibsramen commented Jan 17, 2020 •

edited

Loading

fedarko commented Feb 5, 2020 •

edited

Loading