Closed laxamanaj closed 1 year ago
Hi @laxamanaj, I've not managed to successfully batch run the programs yet as I'm getting a "Rscript command not found" error, so I've not completed a full review as of yet.
From my initial review, I did notice that we have the following files in our repo:
- _pkgdown.yml
- DESCRIPTION
- NEWS.md
Since we are now longer structuring as an R package, I don't think these files are needed anymore.
Also, I'm not sure what others are used to, but should we separate our production and QC scripts? I.e. create a QC folder within submissions/programs for storing QC scripts.
Thanks for noting that you weren't able to run the batch script. I realized that I didn't include the suffix of the script file name. I've updated step 5 above with ./submission/batch_run.sh
. Please give a try again. Thanks.
As for
Bullets 1 and 3 are still tied to our Pilot 3 website, where I don't want to break those quite yet until we have a quarto web page in place. As for the DESCRIPTION file, this is being called by our inst/startup.R
, which is sourced in our .Rprofile
. I will try removing this call in our .Rprofile. If all still works, I will then delete the description file.
Thanks for noting that you weren't able to run the batch script. I realized that I didn't include the suffix of the script file name. I've updated step 5 above with
./submission/batch_run.sh
. Please give a try again. Thanks.As for
- _pkgdown.yml
- DESCRIPTION
- NEWS.md
Bullets 1 and 3 are still tied to our Pilot 3 website, where I don't want to break those quite yet until we have a quarto web page in place. As for the DESCRIPTION file, this is being called by our
inst/startup.R
, which is sourced in our.Rprofile
. I will try removing this call in our .Rprofile. If all still works, I will then delete the description file.
Thanks for the update @laxamanaj. The issue I was getting was R not recognising the "Rscript" commands within the batch script, but @kaz462 kindly provided a solution, which was to open another terminal. I've batched it successfully now and everything is running fine for me.
Thank you @laxamanaj, @kaz462, and @DeclanHodges.
Attached is the log trace of the batch_run shell script (suffix appended) from POSIT cloud (new) terminal (R v4.2.3). In my test runs the log shows 6 errors and 7 warnings (consistently) with .PDF output for tlf-kmplot-pilot3.pdf, .RTF output for tlf-primary-pilot3.rtf and tlf-efficacy-pilot3.rtf, and .OUT output for tlf-demographic-pilot3.out. 107_log_trace_batch_run_script_05312023.txt
Thanks @robertdevine , for attaching a shell script log (side question, was there a certain command for you to save the batch run log?).
Reviewing the log, I see the main Errors were just due to {pilot3}
not being loaded on your local. Before you ran the batch script did you install the {pilot3}
package, per step 2 in the initial comment?
Also, I couldn't find any warnings in this log. Am I missing something?
Thanks @laxamanaj.
Thanks for noting that you weren't able to run the batch script. I realized that I didn't include the suffix of the script file name. I've updated step 5 above with
./submission/batch_run.sh
. Please give a try again. Thanks. As for
- _pkgdown.yml
- DESCRIPTION
- NEWS.md
Bullets 1 and 3 are still tied to our Pilot 3 website, where I don't want to break those quite yet until we have a quarto web page in place. As for the DESCRIPTION file, this is being called by our
inst/startup.R
, which is sourced in our.Rprofile
. I will try removing this call in our .Rprofile. If all still works, I will then delete the description file.Thanks for the update @laxamanaj. The issue I was getting was R not recognising the "Rscript" commands within the batch script, but @kaz462 kindly provided a solution, which was to open another terminal. I've batched it successfully now and everything is running fine for me.
Thanks, @DeclanHodges and thanks @kaz462 for the support.
Thank you @laxamanaj, @kaz462, and @DeclanHodges. Attached is the log trace of the batch_run shell script (suffix appended) from POSIT cloud (new) terminal (R v4.2.3). In my test runs the log shows 6 errors and 7 warnings (consistently) with .PDF output for tlf-kmplot-pilot3.pdf, .RTF output for tlf-primary-pilot3.rtf and tlf-efficacy-pilot3.rtf, and .OUT output for tlf-demographic-pilot3.out. 107_log_trace_batch_run_script_05312023.txt
Thanks @robertdevine , for attaching a shell script log (side question, was there a certain command for you to save the batch run log?).
Reviewing the log, I see the main Errors were just due to
{pilot3}
not being loaded on your local. Before you ran the batch script did you install the{pilot3}
package, per step 2 in the initial comment?Also, I couldn't find any warnings in this log. Am I missing something?
Thanks @laxamanaj.
- Confirmed, Step 2 [the pilot3 library install] suppresses the 6 (no Pilot3 library) Error messages. (a)The bash shell script run 'output' includes: 1 PDF file (tlf-kmplot-pilot3), 2 RTF files (tlf-primary-pilot3, tlf-efficacy-pilot3) , 1 OUT file (tlf-demographic-pilot3)
- Attached is an image of a warning captured (7 Warning messages altogether) in the log trace with vi using the '/' operator.
I see. Many thanks, @robertdevine . These warnings seem to be the result of finding differences from our qc programs, which are acceptable per our QC findings. In the end we won't be submitting the QC programs, so should be ok. I will complete the merge now.
Many thanks for all of your reviews @kaz462 @DeclanHodges @robertdevine
R CMD Check warnings with recent push. Since this repo is not an R package will ignore, but will acknowledge here :
Updated and restructured the analysis repo so that all programs are now under the submission folder.
sdtm
folder is now in the submission folder as if we were to emulate a study that would submit SDTM data as well.programs
folder, where I've updated all scripts to read the source dependencies from the new structure under thecloud/project/submission
directory. ---- The ADaMs should now output to theadam
folder. ---- The TLFs should now output to theoutput
folderFor your review, please do a pull of branch 107 then follow these instructions :
./submission/batch_run.sh
This should then run all of the ADaMs, QC and TLFs in a simple batch run. For now this should work for our needs. I can focus on fancier snakemake scripts later.
If everything runs ok on your end, please approve your review so that we can merge to
main
then start bringing over our files into the eCTD repo and @robertdevine can update #59 .