VirologyCharite / SARS-CoV-2-VL-paper

Data and files from the May 25, 2021 SARS-CoV-2 viral load and infectiousness paper
9 stars 2 forks source link

Make reproduction code work #9

Closed sbfnk closed 2 years ago

sbfnk commented 2 years ago

Fixes all errors and warnings when rendering ExtendedMethods.Rmd from a bare clone of this repo, making the analysis fully reproducible. Visual inspection suggests the results are indeed fully reproduced.

sbfnk commented 2 years ago

Fixes #5.

terrycojones commented 2 years ago

Hi Sebastian - thanks! This looks good to me, but I need to leave it to @gbiele to have a look (as you might have guessed, based on the /Users/guidobiele :-)).

I just realized that I have run into your name before, Christian Drosten mentioned you just before or after xmas.

Thanks again!

gbiele commented 2 years ago

Thanks for the effort Sebastian! I'll look at this tomorrow.

sbfnk commented 2 years ago

Thanks for reviewing this - fyi as far as I can see your comments are not appearing in-line so it's not 100% clear what they're referring to but I can guess.

On your comments so far:

I think this is, like some of the other submitted changes to the code, in order to avoid warning messages, and not because the NA lead downstream to errors.

Yes, sorry the PR contains a mixture of the two which is perhaps not ideal.

I don't mind removing this. but the type argument may be needed on some platforms (if I remember correctly i added it when doing things on linux)

I ran on linux and got an error - the proposed changes are from my attempts to reverse engineer what may have been the intention (see changes to the type/device arguments further down).

gbiele commented 2 years ago

@sbfnk Sorry, I did not see that my comments are not linked to the relevant lines. I'm trying to figure this out now!

gbiele commented 2 years ago

Fixes #5. @sbfnk Thanks again for all the effort Sebastian! I think we can merge this. My only question is if the comment above is just a summary of the effects of all the commits you made, or if this refers to the uploading of posterior samples we discusses here: https://github.com/VirologyCharite/SARS-CoV-2-VL-paper/issues/5. I don't mind also uploading the posterior samples, but I'm not sure if this amount of data would be covered by the plan under which this repo is hosted. What do you think, @terrycojones?

sbfnk commented 2 years ago

I didn’t add any posterior samples so I only added the comment so that issue gets closed. Arguably making the Rmd runnable removes the need for adding the samples (which could still be of benefit but a GitHub repo possibly not an appropriate platform for storage).