Closed dpc10ster closed 5 years ago
This is the output of the R CMD check on my machine, with the offending lines quoted:
==> Rcpp::compileAttributes()
==> devtools::check(args = c('--no-build-vignettes'))
Updating RJafroc documentation Writing NAMESPACE Loading RJafroc Writing NAMESPACE ── Building ─────────────────────────────────────────────────────────────────── RJafroc ── Setting env vars: ● CFLAGS : -Wall -pedantic -fdiagnostics-color=always ● CXXFLAGS : -Wall -pedantic -fdiagnostics-color=always ● CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always ────────────────────────────────────────────────────────────────────────────────────────── ✔ checking for file ‘/Users/Dev/RJafroc/DESCRIPTION’ ... ─ preparing ‘RJafroc’: (2.7s) ✔ checking DESCRIPTION meta-information ... ─ cleaning src ─ installing the package to build vignettes ✔ creating vignettes (48.1s) ─ cleaning src ─ checking for LF line-endings in source and make files and shell scripts (735ms) ─ checking for empty or unneeded directories ─ looking to see if a ‘data/datalist’ file should be added ─ building ‘RJafroc_1.1.1.9000.tar.gz’
── Checking ─────────────────────────────────────────────────────────────────── RJafroc ── Setting env vars: ● _R_CHECK_CRAN_INCOMING_USEASPELL: TRUE ● _R_CHECK_CRAN_INCOMINGREMOTE : FALSE ● _R_CHECK_CRANINCOMING : FALSE ● _R_CHECK_FORCESUGGESTS : FALSE ── R CMD check ─────────────────────────────────────────────────────────────────
* checking package dependencies ...Warning: unable to access index for repository https://CRAN.R-project.org/src/contrib: cannot open URL 'https://CRAN.R-project.org/src/contrib/PACKAGES' OK
- checking if this is a source package ... OK
Status: OK
── R CMD check results ───────────────────────────────── RJafroc 1.1.1.9000 ──── Duration: 7m 24.6s
0 errors ✔ | 0 warnings ✔ | 0 notes ✔
R CMD check succeeded
Thanks Dev. I have been following the updates in your development branch over the last day or two. I will test on my machines tomorrow. I'll take a look at your other points over the next few hours.
Merging #17 into master will decrease coverage by
11.12%
. The diff coverage is68.3%
.
@@ Coverage Diff @@
## master #17 +/- ##
===========================================
- Coverage 79.29% 68.16% -11.13%
===========================================
Files 56 56
Lines 7307 7457 +150
===========================================
- Hits 5794 5083 -711
- Misses 1513 2374 +861
Impacted Files | Coverage Δ | |
---|---|---|
R/UtilPseudoValues.R | 100% <ø> (ø) |
:arrow_up: |
R/StSignificanceTestingCrossedModalities.R | 0% <0%> (-82.54%) |
:arrow_down: |
R/StSignificanceTesting.R | 73.57% <100%> (-13.85%) |
:arrow_down: |
R/StSignificanceTestingSingleFixedFactor.R | 61.22% <50%> (-34.83%) |
:arrow_down: |
R/Compare3ProperRocFits.R | 32.07% <66.66%> (+0.86%) |
:arrow_up: |
R/UtilMeanSquares.R | 84.44% <90.9%> (-4.02%) |
:arrow_down: |
R/DfExtractDataset.R | 54.76% <0%> (-38.1%) |
:arrow_down: |
R/UtilOutputReport.R | 59.57% <0%> (-27.41%) |
:arrow_down: |
R/UtilLesionDistribution.R | 75% <0%> (-25%) |
:arrow_down: |
... and 8 more |
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 9cb7672...53455ef. Read the comment docs.
*** checking package dependencies ...
Warning: unable to access index for repository https://CRAN.R-project.org/src/contrib:
cannot open URL 'https://CRAN.R-project.org/src/contrib/PACKAGES'
CRAN servers are down (an IP issue I believe). This would account for the error and the timeout on the Travis build. Might be worth waiting until CRAN servers are accessible again before making any more changes based on Travis feedback.
Thanks; I will try again tomorrow; Dev
On Wed, Jun 26, 2019 at 5:54 PM Peter Phillips notifications@github.com wrote:
*** checking package dependencies ... Warning: unable to access index for repository https://CRAN.R-project.org/src/contrib: cannot open URL 'https://CRAN.R-project.org/src/contrib/PACKAGES'
CRAN servers are down (an IP issue I believe). This would account for the error and the timeout on the Travis build. Might be worth waiting until CRAN servers are accessible again before making any more changes based on Travis feedback.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRGTF3OCVOHF6W7RVNDP4PQTFA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYU5XPY#issuecomment-506059711, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJREHUOLSNNVH5AKG5WDP4PQTFANCNFSM4H3VFXRQ .
Happy to Skype at some point over the next few days - I will make sure my Skype is switched when I am available.
Also, please review the
cran-comments.md
and summarize your fixes to the Solaris problem?
The cran-comments.md
file mentions a few of the errors I have seen, but as pointed out, they are known bugs.
Solaris fix: Renamed RJafroc C++ erf
function, so as not to clash with an existing function in C++ library with the same name.
Commit 9b622ff fails for me. There is a difference in decimal places and rounding. The test fail output is shown below.
test-compare-3fits.R:15: failure: Compare3ProperRocFits
`Compare3ProperRocFits(1, 1, reAnalyze = TRUE)` has changed from known value recorded in './tempValues/Compare3ProperRocFits01'.
110/3061 mismatches
x[16]: "-3.085018 0.763543 1.331231 1.816872 2.415074 "
y[16]: "-3.085020 0.763543 1.331231 1.816872 2.415074 "
x[43]: " mu lambdaP nuP zeta1 zeta2"
y[43]: " mu lambdaP nuP zeta1 zeta2"
x[44]: "mu 0.079491666 0.006651657 -0.0057395067 0.120288482 0.020959236"
y[44]: "mu 0.079491665 0.006651656 -0.005739507 0.120288433 0.020959235"
x[45]: "lambdaP 0.006651657 0.017861919 -0.0023488117 0.234434954 0.003210050"
y[45]: "lambdaP 0.006651656 0.017861915 -0.002348812 0.234434854 0.003210048"
x[46]: "nuP -0.005739507 -0.002348812 0.0055748260 0.002969605 0.001663961"
y[46]: "nuP -0.005739507 -0.002348812 0.005574826 0.002969603 0.001663961"
Thanks for this; it passed my Travis and local tests; this function uses MLE which can converge to slightly different values as randomness is involved; I will try setting a seed and if that doesn't work I will abandon expect_known_output for this test;
On Fri, Jun 28, 2019 at 3:46 PM Peter Phillips notifications@github.com wrote:
Commit 9b622ff https://github.com/dpc10ster/RJafroc/commit/9b622ffae631b36d8689e46a18aa317ea44ff563 fails for me. There is a difference in decimal places and rounding. The test fail output is shown below.
test-compare-3fits.R:15: failure: Compare3ProperRocFits
Compare3ProperRocFits(1, 1, reAnalyze = TRUE)
has changed from known value recorded in './tempValues/Compare3ProperRocFits01'. 110/3061 mismatches x[16]: "-3.085018 0.763543 1.331231 1.816872 2.415074 " y[16]: "-3.085020 0.763543 1.331231 1.816872 2.415074 "x[43]: " mu lambdaP nuP zeta1 zeta2" y[43]: " mu lambdaP nuP zeta1 zeta2"
x[44]: "mu 0.079491666 0.006651657 -0.0057395067 0.120288482 0.020959236" y[44]: "mu 0.079491665 0.006651656 -0.005739507 0.120288433 0.020959235"
x[45]: "lambdaP 0.006651657 0.017861919 -0.0023488117 0.234434954 0.003210050" y[45]: "lambdaP 0.006651656 0.017861915 -0.002348812 0.234434854 0.003210048"
x[46]: "nuP -0.005739507 -0.002348812 0.0055748260 0.002969605 0.001663961" y[46]: "nuP -0.005739507 -0.002348812 0.005574826 0.002969603 0.001663961"
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRE4HQH6566OM535NQTP4ZTB7A5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3ADFI#issuecomment-506855829, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRHPTADFAA5KDMQQVKDP4ZTB7ANCNFSM4H3VFXRQ .
Is it tested by Travis?
I think the test might be skipped.
https://www.rdocumentation.org/packages/testthat/versions/2.1.1/topics/skip
You are absolutely right; it hangs up Travis due to long computation time; so it probably would have failed; could you try the new one with fixed seed = 1?
On Jun 28, 2019, at 4:00 PM, Peter Phillips notifications@github.com wrote:
Is it tested by Travis?
https://github.com/dpc10ster/RJafroc/blob/9b622ffae631b36d8689e46a18aa317ea44ff563/tests/testthat/test-compare-3fits.R#L4 https://github.com/dpc10ster/RJafroc/blob/9b622ffae631b36d8689e46a18aa317ea44ff563/tests/testthat/test-compare-3fits.R#L4 I think the test might be skipped. https://www.rdocumentation.org/packages/testthat/versions/2.1.1/topics/skip https://www.rdocumentation.org/packages/testthat/versions/2.1.1/topics/skip — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRGHOGZFDAR6SBI22T3P4ZUVXA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3BASY#issuecomment-506859595, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRASO7LVPVLGHUWDGVTP4ZUVXANCNFSM4H3VFXRQ.
Test fail again here with the same rounding and decimal place errors.
I think we need a more detailed test to step through the values (or groups of values) in the output object, using expect_equal to overlook any rounding error.
https://www.rdocumentation.org/packages/testthat/versions/2.1.1/topics/equality-expectations
Differences in Compare3ProperRocFits01 can be found here:
https://gist.github.com/pwep/619a823c7d2d4d4dfc489e73bcdec83c/revisions
I am going to push shortly; could you test out new version; i expect it to pass
On Jun 28, 2019, at 4:21 PM, Peter Phillips notifications@github.com wrote:
Differences in Compare3ProperRocFits01 can be found here:
https://gist.github.com/pwep/619a823c7d2d4d4dfc489e73bcdec83c/revisions https://gist.github.com/pwep/619a823c7d2d4d4dfc489e73bcdec83c/revisions — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRCLPKCA5EJNQ2ZX2V3P4ZXDFA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3CKXY#issuecomment-506864991, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRGGPHXBDNWCM6TZJHTP4ZXDFANCNFSM4H3VFXRQ.
Errors, but this time only of file length on CompareProperRocFits01
and CompareProperRocFits02
.
CompareProperRocFits01
does not contain the complete $allBinnedDatasets[[1]]$NL
array
I have an error in the file on line 1813.
[ reached getOption("max.print") -- omitted 85 matrix slice(s) ]
OK; I am giving up on this approach; thanks for trying them out on Windows, I presume; dev
On Jun 28, 2019, at 4:36 PM, Peter Phillips notifications@github.com wrote:
Errors, but this time only of file length on CompareProperRocFits01 and CompareProperRocFits02.
CompareProperRocFits01 does not contain the complete $allBinnedDatasets[[1]]$NL array I have an error in the file on line 1813. [ reached getOption("max.print") -- omitted 85 matrix slice(s) ]
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRHR47GBCJ2SQCZYLFDP4ZY5NA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3DLGI#issuecomment-506869145, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRB3EJIDHKSHODEYP2LP4ZY5NANCNFSM4H3VFXRQ.
All tests performed on Linux with R 3.6.0, run with devtools::test()
Thanks for update;
On Jun 28, 2019, at 4:42 PM, Peter Phillips notifications@github.com wrote:
All tests performed on Linux with R 3.6.0, run with devtools::test()
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRCEOYU2AMKTXT2WXGTP4ZZTZA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3DXGA#issuecomment-506870680, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRCSAK3VOFYJMF4KDTLP4ZZTZANCNFSM4H3VFXRQ.
CompareProperRocFits02
has extra lines, based on how $allDatasetsResults[[1]][[8]]$retCbm$covMat
is written to the file. Difference between files is shown at:
https://gist.github.com/pwep/d3d50bd94a33ea951aa8e59b7e65d563/revisions
There must be line-length default which is different on your platform compared to mine.
I agree; I am coming to the conclusion that expect_known_output() is NOT one of Hadley Wickham's more useful creations
I can't tell you how much time I have wasted on it.
Actually, if I removed the covariance matrices from the final output, not a difficult step, that might (??) solve the problem; should I waste some more time? ha ha
Dev
On Fri, Jun 28, 2019 at 4:49 PM Peter Phillips notifications@github.com wrote:
CompareProperRocFits02 has extra lines, based on how $allDatasetsResults[[1]][[8]]$retCbm$covMat is written to the file. Difference between files is shown at: https://gist.github.com/pwep/d3d50bd94a33ea951aa8e59b7e65d563/revisions
There must be line-length default which is different on your platform compared to mine.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRB4JAJTPXHNPHBXLELP4Z2ORA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3EF3A#issuecomment-506872556, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRGLPG6N7H674INZTMDP4Z2ORANCNFSM4H3VFXRQ .
I'll have a look at the testthat
API about checking equality across objects, rather than files. If we have an R data file we know is correct, there must be a way of loading it, and checking new output against it (within certain tolerances.)
OK; BTW, I am using 4 character extensions in some of the saved “good value” files in Git (for later comparison); will that break in some operating systems? I know if will break in Windows XP
On Jun 28, 2019, at 5:20 PM, Peter Phillips notifications@github.com wrote:
I'll have a look at the testthat API about checking equality across objects, rather than files. If we have an R data file we know is correct, there must be a way of loading it, and checking new output against it (within certain tolerances.)
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRDBFF3JUICLUBP3ZOLP4Z6AHA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3GCYQ#issuecomment-506880354, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRHDD2O7LQJ4GTYRE6LP4Z6AHANCNFSM4H3VFXRQ.
Office uses 4 character extensions for well-known file types: .docx
, .xlsx
, etc. Windows XP should handle it (and other operating systems too).
Saving an R object as as an RDS file should be safe across platforms.
I have created a small example file which outlines the approach. Very similar to what we have already, but the testing files are in RDS format.
https://gist.github.com/pwep/a8f35093e40c228e91d0d90b90c235bd
Files such as CompareProperRocFit01
in the goodValues
directory would be saved as RDS files. A testthat
test would readRDS()
that file into memory. A simple expect.equal(..., tolerance=0.00001)
would be able to check the goodValues
version against the test-generated version.
One test fails on Linux:
✖ | 5 1 | Fitting routines [1607.0 s]
──────────────────────────────────────
test-fitting.R:97: failure: FitRsmRoc
FitRsmRoc(dataset02, UtilLesionDistribution(dataset02))[1:8] not equal to `ret`.
Component "mu": Mean relative difference: 3.865783e-08
Component "lambdaP": Mean relative difference: 2.563811e-08
Component "zetas": Mean relative difference: 2.05855e-07
──────────────────────────────────────
A tolerance value in test-fitting.R
will fix it.
Thanks; I applied your fix and will push soon;
Travis failed for developer version of R.
I am attaching the log file; the relevant part appears to be as follows: I think I need to set attribute explicitly; is that your understanding?
Running the tests in ‘tests/testthat.R’ failed. Last 13 lines of output: Component "ciDiffTrtRRRC": Component "Estimate": Attributes: < target is NULL, current is list > Component "ciDiffTrtRRRC": Component "Estimate": target is numeric, current is array Component "ciAvgRdrEachTrtRRRC": Component "StdErr": Attributes: < target is NULL, current is list > Component "ciAvgRdrEachTrtRRRC": Component "StdErr": target is numeric, current is array Component "ciAvgRdrEachTrtRRRC": Component "DF": Attributes: < target is NULL, current is list > Component "ciAvgRdrEachTrtRRRC": Component "DF": target is numeric, current is array Component "ciDiffTrtFRRC": Component "Treatment": Attributes: < target is NULL, current is list > ...
OK: 165 SKIPPED: 6 WARNINGS: 0 FAILED: 1
On Jul 1, 2019, at 05:29, Peter Phillips notifications@github.com wrote:
One test fails on Linux:
✖ | 5 1 | Fitting routines [1607.0 s] ────────────────────────────────────── test-fitting.R:97: failure: FitRsmRoc FitRsmRoc(dataset02, UtilLesionDistribution(dataset02))[1:8] not equal to
ret
. Component "mu": Mean relative difference: 3.865783e-08 Component "lambdaP": Mean relative difference: 2.563811e-08 Component "zetas": Mean relative difference: 2.05855e-07 ────────────────────────────────────── A tolerance value in test-fitting.R will fix it.— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRBJDCDUPLWLE47MXI3P5HE6DA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY5RT7A#issuecomment-507189756, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRGIGAQQEDARFYY2R43P5HE6DANCNFSM4H3VFXRQ.
Replacing expect_equal with expect_equivalent may fix this problem as it ignores attributes in testing equality. I have just pushed.
On Mon, Jul 1, 2019 at 10:17 AM Dev Chakraborty dpc10ster@gmail.com wrote:
Thanks; I applied your fix and will push soon;
Travis failed for developer version of R.
I am attaching the log file; the relevant part appears to be as follows: I think I need to set attribute explicitly; is that your understanding?
Running the tests in ‘tests/testthat.R’ failed. Last 13 lines of output: Component "ciDiffTrtRRRC": Component "Estimate": Attributes: < target is NULL, current is list > Component "ciDiffTrtRRRC": Component "Estimate": target is numeric, current is array Component "ciAvgRdrEachTrtRRRC": Component "StdErr": Attributes: < target is NULL, current is list > Component "ciAvgRdrEachTrtRRRC": Component "StdErr": target is numeric, current is array Component "ciAvgRdrEachTrtRRRC": Component "DF": Attributes: < target is NULL, current is list > Component "ciAvgRdrEachTrtRRRC": Component "DF": target is numeric, current is array Component "ciDiffTrtFRRC": Component "Treatment": Attributes: < target is NULL, current is list > ...
OK: 165 SKIPPED: 6 WARNINGS: 0 FAILED: 1
- Failure: StSignificanceTestingCrossedModalities (@test-significance-testing.R#105)
On Jul 1, 2019, at 05:29, Peter Phillips notifications@github.com wrote:
One test fails on Linux:
✖ | 5 1 | Fitting routines [1607.0 s] ────────────────────────────────────── test-fitting.R:97: failure: FitRsmRoc FitRsmRoc(dataset02, UtilLesionDistribution(dataset02))[1:8] not equal to
ret
. Component "mu": Mean relative difference: 3.865783e-08 Component "lambdaP": Mean relative difference: 2.563811e-08 Component "zetas": Mean relative difference: 2.05855e-07 ──────────────────────────────────────A tolerance value in test-fitting.R will fix it.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/17?email_source=notifications&email_token=AH4NJRBJDCDUPLWLE47MXI3P5HE6DA5CNFSM4H3VFXR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY5RT7A#issuecomment-507189756, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRGIGAQQEDARFYY2R43P5HE6DANCNFSM4H3VFXRQ .
Closing this pull request after running on all 20 platforms on rhub
merged development branch into master
I have pushed my edits to the
testthat
folder. With about 560 tests, I believe it is time to move on to CRAN submission process and final testing. The main work remaining here is to bring the standard of the code in all test files to that intest-significance-testing.R
which I can do for the next submission.Can you review the
DESCRIPTION
file? Particularly the authors. You have played a critical part in this project and I want to give you full credit. Also, please review thecran-comments.md
and summarize your fixes to the Solaris problem? And theREADME.md
- I need to include your contributions here. All edits will be much appreciated. No rush, just give me a general idea.RJafroc
passes all tests on my OSX machine but did generate an unusual warning, which is not listed in the final summary as a warning; strange; the relevant lines are shown below in my next commentCan you run tests on your machines?
When you feel it is appropriate, I suggest another Skype call, as some things are best discussed that way.
I replaced
options(warn = 2)
but closed that file (Compare3RocFits.R
) with a final call withoptions(warn = 0)