Add to README; bump / pin TF versions; other minor fixes #109
Conversation
- gender -> sex in the example formula - clarify difference from R formulas - add example of categorical variables being difficult
Ideally the error message given would be better, but I guess that's a future TODO.
ideally we should be using 1.15.2 for both conda and pip, but conda-forge doesn't seem to have 1.15.2 yet. hence the slightly different minimum versions. (... not sure if this will work since conda-forge's latest tf v1 build seems to be for 1.13? we'll see on travis.) i have verified on an ubuntu computer that the tests pass with tf 1.15.2 (installed via pip)
"if tqdm is so good why isn't there a tqdm 2"
-Some of the invocations of "songbird multinomial" (either inside or outside of QIIME 2) had inconsistent parameters. Examples include some not specifying a differential prior, some not specifying a training column, and some specifying 5000 instead of 10000 epochs. IMO it's important to be consistent here (esp. for things like comparing model fitting across runs). -Added more structure to section 6 (validating by comparing...) -- in particular, shuffled around the standalone and QIIME 2 sections to resemble the earlier parts of the README -Added the "YOU NEED TO RUN A NULL MODEL" sentence to the QIIME 2 section, and bolded the sentence in both the standalone and QIIME 2 sections to make it even clearer to users -Fixing some minor typos, wording issues, etc.
fedarko
changed the title
since we've defined both of these as distinct terms in the readme
|
|
||
| # Visualize the first model's regression stats *and* the baseline model's | ||
| # Visualize the first model's regression stats *and* the null model's |
There was a problem hiding this comment.
The null model terminology is fine.
But one thing to note here is that baseline can also refer to other models. For instance, you wanted to look at just the contribution of disease, you could have model1_formula='sex + disease' and model2_formula='sex' and the Q2 can tell you how much better model1 performed compared to model2 (allowing one to determine how explanatory disease is).
We can change the terminology, but if we do that, we should hint that these sorts of examples.
There was a problem hiding this comment.
Gotcha -- yes, that's part of the reason I changed this in the first place. The README defines "baseline models" and "null models" as being separate things, and it was previously mixing up the terms -- so to make things clearer (and make sure that the outputs from running one command wouldn't get overwritten by another command) I changed up the first part of this to say null instead of baseline models. ... If that makes sense.
|
@fedarko that's awesome! I'm overall happy with the changes. Thanks for putting this together! |
- mentioning lack of parity between versions right now (biocore#113) - clarifying that Q^2 should be at least 0 (linked to q2 forum post)
|
Thanks @mortonjt! I made a few more updates to the README re our discussion today, let me know if things seem good. Once this is in it might be worth trying to get a release out eventually -- there have been a lot of features added in recently that it'd be good to get out to everyone. but no rush of course :) |
|
Thanks @fedarko ! Yes, let's get a release out soon (maybe even next week). |
For some reason updating the TensorFlow versions caused the build to start passing out of nowhere (even without using the fixes outlined in #107??), so i have no idea why that's happening but I guess I'm not gonna complain.