dpc10ster / RJafroc

Artificial Intelligence: Evaluating AI, optimizing AI
19 stars 8 forks source link

HrSe #90

Closed dpc10ster closed 1 year ago

dpc10ster commented 1 year ago

Email from an user cited inconsistent values of K (total number of cases) when using code with FOM = "HrSe" (or FOM = "HrSp"). Edited excerpts from email follow:

...issue with RJafroc v2.1.2: function StSignificanceTesting, with FOM = “HrSe”, method = ”DBM” and analysisOption = "RRRC", calls function UtilVarComponentsDBM where K is calculated using the length of the third dimension of the pseudo-value array, which in this case is the total number of diseased cases, i.e., K = K2. This is correct as only diseased cases contribute to sensitivity. However, when calculating the statistics using function DBMSummaryRRRC() then K = K1+K2, i.e., the total number of cases, which is incorrect.

This bug affects all FOMs that depend on only diseased or only non-diseased cases, fortunately a rare usage of the software.

Here is the relevant history of the software:

There was a change in maintainer between version 0.0.1 and all subsequent versions. The first version was was coded by a excellent programmer (Xuetong Zhai). However his coding style was different from Dr. Chakraborty which was making it difficult for him to maintain and extend the code.

The StSignificanceTesting function was extensively recoded. To guard against errors creeping I inserted 'testthat' testing functions that compared the new values against the original values. The comparisons depended on the dataset, the figure of merit and the analysis method, see skeleton code below. Only the most commonly used FOMs were tested and unfortunately these did not include all FOMs that depend on only diseased or only non-diseased cases (these FOMs are not recommended by Dr. Chakraborty but some users still need them due to FDA imposed requirement).

Following the email the original code (which had a few remaining bugs) was corrected. The current code was also corrected to remove the discrepancy noted in the email and other bugs. Finally testthat function tests were included to check the consistency between the corrected original code and the current version. Here is the skeleton code showing the testing conditions:

  dataset_arr <- c("dataset02", "dataset05") 
  FOM_arr <- c("Wilcoxon", "HrAuc", "wAFROC","AFROC", "wAFROC1","AFROC1",
               "MaxLLF","MaxNLF","MaxNLFAllCases", "ExpTrnsfmSp", "HrSp", "HrSe")
...
...
analysisOption = "RRRC"
...
...
analysisOption = "FRRC"
...
...
analysisOption = "RRFC"

The two testing functions are (you need to fork the repository to see these functions):

~/GitHub/RJafroc/tests/testthat/test-OldVsNew-DBM.R for the DBM method

and

~/GitHub/RJafroc/tests/testthat/test-OldVsNew-OR.R for the OR method.

All corrections have been pushed to the master branch. The corrected old code is still there but is no longer visible in the HTML documentation. It is in the following file:

~/GitHub/RJafroc/R/StOldCode.R

A new CRAN submission is being prepared that will implement all bug-fixes. Anticipated submission date is approximately 9/30/2023. Until then users should download or fork the master branch.

I appreciate the sender of the email for the effort that went into detection of this bug and would encourage anyone to closely examine the code. All significant bug detections will be acknowledged.

dpc10ster commented 1 year ago

Some of the updated code has been independently confirmed in SAS by the user. I am closing this issue.