weecology / LDATS

Latent Dirichlet Allocation coupled with Bayesian Time Series analyses
https://weecology.github.io/LDATS
Other
25 stars 5 forks source link

Paper comparison #105

Closed diazrenata closed 5 years ago

diazrenata commented 5 years ago

Here's a draft of the paper comparison vignette. Feedback would be welcome! But I'll also continue to comb through it, so no worries if it's not at the top of your list.

codecov-io commented 5 years ago

Codecov Report

Merging #105 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #105   +/-   ##
=======================================
  Coverage   96.77%   96.77%           
=======================================
  Files          10       10           
  Lines        1024     1024           
=======================================
  Hits          991      991           
  Misses         33       33

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4939297...884fedc. Read the comment docs.

codecov-io commented 5 years ago

Codecov Report

Merging #105 into master will increase coverage by 0.4%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #105     +/-   ##
=========================================
+ Coverage   96.77%   97.18%   +0.4%     
=========================================
  Files          10       10             
  Lines        1024     1030      +6     
=========================================
+ Hits          991     1001     +10     
+ Misses         33       29      -4
Impacted Files Coverage Δ
R/LDA_TS.R 100% <ø> (ø) :arrow_up:
R/TS_on_LDA.R 99.31% <ø> (ø) :arrow_up:
R/TS.R 98.58% <100%> (ø) :arrow_up:
R/utilities.R 100% <100%> (+9.09%) :arrow_up:
R/LDA_TS_plots.R 94.44% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 10bceb1...111e8e3. Read the comment docs.

juniperlsimonis commented 5 years ago

hmmm, i'm getting an error when i run devtools::build_vignettes()

"Access violation in generated code" and "pandoc document conversion failed with error 11"

are you able to build the vignette if you update your local copy to the current state of this branch?

juniperlsimonis commented 5 years ago

I took a look through the text and in general it looks really great! Thanks for all of your work on this. My one thought would be that it might be helpful to have an "overall" comparison/side-by-side between the two methods, maybe as a table or something? just so that it's quick and easy to see what the differences are. so like each row would be a part of the model and there'd be two columns, one for each methodology. it'd be fine to have stuff that's similar between the two models as well as the things that are different. does that make sense?

otherwise, i think this is in a really good spot, and we just need to figure out what's up with rendering it

diazrenata commented 5 years ago

Thanks for looking it over, and definitely on the table!

The rendering is a puzzle. I just did git pull + devtools::build_vignettes and it rendered fine on my computer. It copies everything and stores the rendered version in the doc folder, which is usually ignored by git. This commit is pushing the rendered version so you can at least look at it.

I'll have more time later to dig into exactly what's up, but my first thought is it might have to do with using the here package (since that does a lot of scampering up and down paths and might run into authorization issues)? But really that's just a guess.

juniperlsimonis commented 5 years ago

ooh, thanks for passing the rendered version along! yeah that's probably the situation, something to do with where the files are, etc.

if you have the capacity, i think what might be the best is to have the vignettes not actually running any of the code, but really just condense everything into a workspace (RData) file or something and then have most of the code chunks just be displaying, not actually running (eval =F) so the vignette is showing how the comparison works but isn't actually running anything.

the other thing i didn't edit that could be good to do would be to widen the figures to 7 from 6, like i did in the rodent example vignette, it just helps with the density of the time series

juniperlsimonis commented 5 years ago

FYI, I've updated the default timename to be "time" rather than "newmoon", so that the code is more general by default (newmoon isn't meaningful in many cases). to integrate this change you'll need to rebase and then make sure all of the calls to TS_controls_list now include timename = "newmoon". I've updated it the other vignette and throughout the rest of the codebase, so you only should need to worry about the comparison vignette.

juniperlsimonis commented 5 years ago

addresses #78