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

ENH: Q2 summary interval allow float #108

Merged
merged 7 commits into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ language: python
env:
- PYVERSION=3.5
before_install:
- sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test
gwarmstrong marked this conversation as resolved.
Show resolved Hide resolved
- sudo apt-get update
- sudo apt-get -y install gcc-4.9
- sudo apt-get -y install --only-upgrade libstdc++6
fedarko marked this conversation as resolved.
Show resolved Hide resolved
- export MPLBACKEND='Agg'
- wget -q https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh -O miniconda.sh
- export MINICONDA_PREFIX="$HOME/miniconda"
Expand Down
2 changes: 1 addition & 1 deletion songbird/multinomial.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def fit(self, epochs=10, summary_interval=100, checkpoint_interval=3600):
----------
epochs : int
Number of epochs to train
summary_interval : int
summary_interval : float
Number of seconds until a summary is recorded
checkpoint_interval : int
Number of seconds until a checkpoint is recorded
Expand Down
2 changes: 1 addition & 1 deletion songbird/q2/plugin_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
'clipnorm': Float,
'min_sample_count': Int,
'min_feature_count': Int,
'summary_interval': Int,
'summary_interval': Float,
'random_seed': Int,
},
outputs=[
Expand Down
47 changes: 47 additions & 0 deletions songbird/q2/tests/test_method.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from skbio import OrdinationResults
from skbio.stats.composition import clr, clr_inv
import numpy.testing as npt
import pandas as pd
from songbird.q2.plugin_setup import plugin as songbird_plugin


class TestMultinomial(unittest.TestCase):
Expand Down Expand Up @@ -73,6 +75,51 @@ def test_fit_consistency(self):
npt.assert_array_equal(res_biplot1.samples, res_biplot2.samples)
npt.assert_array_equal(res_biplot1.features, res_biplot2.features)

def test_fit_float_summary_interval(self):
tf.set_random_seed(0)
fedarko marked this conversation as resolved.
Show resolved Hide resolved
md = self.md

multregression = songbird_plugin.actions['multinomial']

md.name = 'sampleid'
md = qiime2.Metadata(md)

exp_beta = clr(clr_inv(np.hstack((np.zeros((2, 1)), self.beta.T))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like this line (as well as a few others towards the end of this function, e.g. the npt.assert_allclose lines) were taken from test_fit() in this file. That's not a problem, but this should be commented to make that clear if possible -- the clr(clr_inv()) stuff in particular is apparently kind of a hack, and we should try to document where it's used so it could ideally be taken care of in the future.

gwarmstrong marked this conversation as resolved.
Show resolved Hide resolved

q2_table = qiime2.Artifact.import_data('FeatureTable[Frequency]',
self.table)

q2_res_beta, q2_res_stats, q2_res_biplot = multregression(
table=q2_table, metadata=md,
min_sample_count=0, min_feature_count=0,
formula="X", epochs=1000,
summary_interval=0.5,
)

try:
gwarmstrong marked this conversation as resolved.
Show resolved Hide resolved
res_biplot = q2_res_biplot.view(OrdinationResults)
except Exception:
raise AssertionError('res_biplot unable to be coerced to '
'OrdinationResults')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was gonna ask you to use a more specific error type than just Exception but I tried it out and, yep, you get a basic Exception when transformations fail. The more you know!

(Maaybe worth adding a comment mentioning this just so that future people reviewing this code don't have the same reaction I just did...)

try:
res_beta = q2_res_beta.view(pd.DataFrame)
except Exception:
raise AssertionError('res_beta unable to be coerced to '
'pd.DataFrame')
try:
res_stats = q2_res_stats.view(qiime2.Metadata)
except Exception:
raise AssertionError('res_stats unable to be coerced to '
'qiime2.Metadata')
fedarko marked this conversation as resolved.
Show resolved Hide resolved

u = res_biplot.samples.values
v = res_biplot.features.values.T
npt.assert_allclose(u @ v, res_beta.values,
atol=0.5, rtol=0.5)

npt.assert_allclose(exp_beta, res_beta.T, atol=0.6, rtol=0.6)
self.assertGreater(len(res_stats.to_dataframe().index), 1)


if __name__ == "__main__":
unittest.main()