`stopTrial-StoppingOrdinal` fails when `StoppingOrdinal` rules are nested

[Puzzled-Face]

stopTrial-StoppingOrdinal fails when StoppingOrdinal rules are nested within the overall stopping rule.

No error occurs with the design returned by .DefaultDesignOrdinal() because its stopping rule is given by

my_stopping1 <- StoppingMinCohorts(nCohorts = 3)
my_stopping2 <- StoppingTargetProb(
  target = c(0.2, 0.35),
  prob = 0.5
)
my_stopping3 <- StoppingMinPatients(nPatients = 20)
my_stopping <- StoppingOrdinal(1L, (my_stopping1 & my_stopping2) | my_stopping3)

Note that the overall rule is a StoppingOrdinal, but none of its components are. To demonstrate:

design <- .DefaultDesignOrdinal()
samples <- mcmc(design@data, design@model, .DefaultMcmcOptions())

answer <- stopTrial(
  stopping = design@stopping,
  dose = Inf,
  samples = samples,
  model = design@model,
  data = design@data
)

attributes(answer) <- NULL
answer
[1] FALSE

But an overall stopping rule that is (at least partially) composed of StoppingOrdinals is perfectly possible:

design@stopping <- StoppingOrdinal(
  1L, # Ignored
  StoppingAny(
    list(
      StoppingOrdinal(1L, StoppingTargetProb(target = c(0.2, 0.4), prob = 0.5)),
      StoppingOrdinal(2L, StoppingTargetProb(target = c(0.0, 0.1), prob = 0.9))
    )
  )
)

Here, we stop the trial if, at the next dose, the chance that the probability of grade 1 toxicity is in the range [0.2, 0.4) is at least 0.5 or the chance that the probability of a grade 2 toxicity is less than 0.1 is at least 90%.

After which,

answer <- stopTrial(
  stopping = design@stopping,
  dose = Inf,
  samples = samples,
  model = design@model,
  data = design@data
)
Error in FUN(X[[i]], ...) :
StoppingOrdinal objects can only be used with LogisticLogNormalOrdinal models and DataOrdinal data objects. In this case, the model is a 'LogisticLogNormal' object and the data is in a Data object.

The problem arises because stopTrial-StoppingOrdinal simply converts the model and data objects it is passed to their binary equivalents and then delegates to the appropriate stopTrial method without checking whether any subordinate Stopping rules are themselves StoppingOrdinals. The situation is relatively complicated because the nesting may not be direct. For example:

stoppingParticipantCount <- StoppingMinPatients(
  nPatients = 60, 
  report_label = "Participant count"
)
stoppingToxic <- StoppingMissingDose(report_label = "All toxic")
stoppingFutility <- StoppingAny(
  list(stoppingParticipantCount, stoppingToxic), 
  report_label = "Futility"
)

stoppingPatientsAtMTD <- StoppingPatientsNearDose(
  nPatients = 20L, 
  percentage = 0, 
  report_label = "Patients at MTD"
)
stoppingSafeDLAE <- StoppingOrdinal(
  1L, 
  StoppingTargetProb(
    target = c(0.0, 0.2), 
    prob = 0.8, 
    report_label = "DLAE-safe"
  )
)
stoppingSafeCRS <- StoppingOrdinal(
  2L, 
  StoppingTargetProb(
    target = c(0.0, 0.05), 
    prob = 0.8, 
    report_label = "CRS-safe"
  )
)

stoppingSuccess <- StoppingAll(
  list(
    stoppingPatientsAtMTD,
    stoppingSafeDLAE,
    stoppingSafeCRS
  ),
  report_label = "Stopping success"
)
trialStopping <- StoppingOrdinal(1L, StoppingAny(list(stoppingFutility, stoppingSuccess)))

We should also check that we don't have similar issues with other rules (eg size-CohortSizeMin and size-CohortSizeMax; maxDose-IncrementsMin etc).

[Puzzled-Face]

Possible solution:

• Write a helper function (h_needs_ordinal_input, perhaps) that
  • returns TRUE for StoppingOrdinal
  • calls itself for each element of (subclasses of) StoppingList and returns TRUE iff any sub call returns TRUE
  • returns FALSE otherwise
• Add a call to this helper at the start of every stopTrial method and convert the input model and data objects if required.

[danielinteractive]

Thanks @Puzzled-Face , I think a principled solution could maybe be to add corresponding stopTrial methods with DataOrdinal signature parts to all relevant Stopping classes. Then, the required "conversion" to binary model and data would only happen at the very end when it is needed.

[Puzzled-Face]

Thanks @Puzzled-Face , I think a principled solution could maybe be to add corresponding stopTrial methods with DataOrdinal signature parts to all relevant Stopping classes. Then, the required "conversion" to binary model and data would only happen at the very end when it is needed.

Yes. That works in the case above.

I believe we will have the same issue with the other Rules classes: CohortSize, Increments and NextBest. If we do this, then I think that the existing wrapper classes (CohortSizeOrdinal and the like) become redundant. Do you agree?

In a number of cases, the model, samples and data method arguments are required only to match the signature of the generic. They are not actually required to evaluate the rule. (CohortSizeConst is an obvious example). In these cases, we have at least three options:

1. Simply copy-and-paste the existing method, modifying the signature to use ordinal equivalents of the current argument classes.
2. Convert the body of the existing method to a helper function. Modify the existing method so that it contains a simple call to the helper. Create an ordinal equivalent method that converts model, samples and data to the required binary equivalent (we already have helpers to do this) and then calls the helper.
3. Modify the signature of the existing method so that samples, model and data are of class ANY and then use checkmate to confirm that they are in the set of permitted classes (eg for the data argument, Data or DataOrdinal are the only acceptable classes).

I think I prefer the final option.

Note that these three options apply only when the corresponding object is not required to evaluate the rule. In other cases, a new method, whose signature uses ordinal classes, will be required. The general form of these new methods will be:

1. Confirm the grade parameter is valid
2. Convert the "ordinal class" arguments to their grade-specific binary equivalents (we already have helpers to do this).
3. Call the binary equivalent of the method

[danielinteractive]

Thanks @Puzzled-Face , I think a principled solution could maybe be to add corresponding stopTrial methods with DataOrdinal signature parts to all relevant Stopping classes. Then, the required "conversion" to binary model and data would only happen at the very end when it is needed.

Yes. That works in the case above.

I believe we will have the same issue with the other Rules classes: CohortSize, Increments and NextBest. If we do this, then I think that the existing wrapper classes (CohortSizeOrdinal and the like) become redundant. Do you agree?

Yeah I guess that sounds right. But before changing everything at once, probably safer to first start with one set of rules and confirm this in practice.

In a number of cases, the model, samples and data method arguments are required only to match the signature of the generic. They are not actually required to evaluate the rule. (CohortSizeConst is an obvious example).

Yes

In these cases, we have at least three options:

1. Simply copy-and-paste the existing method, modifying the signature to use ordinal equivalents of the current argument classes.

We should avoid copy-and-paste when possible, so this is not preferred

2. Convert the body of the existing method to a helper function. Modify the existing method so that it contains a simple call to the helper. Create an ordinal equivalent method that converts model, samples and data to the required binary equivalent (we already have helpers to do this) and then calls the helper.

I think that makes sense.

3. Modify the signature of the existing method so that samples, model and data are of class ANY and then use checkmate to confirm that they are in the set of permitted classes (eg for the data argument, Data or DataOrdinal are the only acceptable classes).

The method dispatch is not just for checking, but also for choosing the right method - so if we were to do this, then internally we need some kind of if/else conditions etc. and this would be unfortunate since we are already in the S4 framework.

I think I prefer the final option.

Note that these three options apply only when the corresponding object is not required to evaluate the rule. In other cases, a new method, whose signature uses ordinal classes, will be required.

Exactly, and for me that is another argument why I would go with option 2 above - for consistency.

The general form of these new methods will be:

1. Confirm the grade parameter is valid
2. Convert the "ordinal class" arguments to their grade-specific binary equivalents (we already have helpers to do this).
3. Call the binary equivalent of the method

Yes, and this can either work with conversions and then method calls, or calling helper functions similarly to above.