pharmaverse / admiralonco

Oncology extension package for ADaM in R Asset Library (admiral)
https://pharmaverse.github.io/admiralonco/index.html
Apache License 2.0
32 stars 8 forks source link

derive_param_confirmed_resp/bor clarify CR->PR case and add assertion #87

Closed rossfarrugia closed 2 years ago

rossfarrugia commented 2 years ago

Please select a category the issue is focused on?

Function Documentation

Let us know where something needs a refresh or put your idea here!

Add documentation around why CR->PR is counted as NE for confirmed, rather than PD as suggested in RECIST1.1 document - rationale is to keep consistency with other functions and endpoints that identify PD such as PFS, where this is seemingly more just a data issue.

Add an assertion check for this case with a warning message to suggest teams to query the data.

rossfarrugia commented 2 years ago

@bundfussr i felt this one best for you as you know the background in depth.

sgorm123 commented 2 years ago

@rossfarrugia @bundfussr I checked at Amgen and this would be a data issue, as expected, so an assertion would be a good idea. We would (as GSK) convert this to be a PD (and add a footnote in the tables/figures) if it remained in the data at the time of the snapshot (i.e. in the study teams I asked, this may not be consistent across Amgen).

Though the footnote example I have is quite detailed, so potentially the assertion text needs to be carefully considered.

bundfussr commented 2 years ago

@sgorm123 , at which point do you convert CR->PR to PD? Do you convert it only temporary for the derivation of confirmed response and BOR? Or do you convert it on the level of the source assessments such that it affects all derived parameters, e.g., also time to first PD?

sgorm123 commented 2 years ago

Just for the response parameters @bundfussr

rossfarrugia commented 2 years ago

@bundfussr sounding like we need to make this an argument? But also I'd recommend to question this with our data standards as seems our approach is the odd one out (although i do get why - tbh there's no perfect solution other than just cleaning the data)

amitjaingsk commented 2 years ago

@bundfussr @rossfarrugia if this can be an argument , I think it can give the functionality to this function, which will be helpful around this case. And assertion will be also be helpful. Please let us know what you decided. Thanks!

bundfussr commented 2 years ago

I assume the updated response values should be taken into account for the restriction to first PD. I.e., CR, PR, PR, PR should result in PD (or SD) because after the update we have CR, PD, PR, PR, which is restricted to CR, PD.

@sgorm123 , @amitjaingsk , do you agree?

rossfarrugia commented 2 years ago

Thanks @sgorm123 - that's how i interpreted the request too but @amitjaingsk mentioned a slightly more complex convention where they would take the subsequent records into account.

I recommend that as discussed at the call for now @bundfussr just adds the assertion for this one to help users identify the data issue, and then we can add the PD bit later once we've had more time to figure out best way to implement to cover all needs.

sgorm123 commented 2 years ago

@rossfarrugia I sent this too soon, I will resend tomorrow after review from Catherine. However, I agree with your comment, add the assertion and add the PD bit later on (this is also handled differently across teams at Amgen, something I wanted to comment on, before I posted).

rossfarrugia commented 2 years ago

ah ok thanks! seems a really complex piece this. the more i hear the more i become inclined to say let's just leave it with the assertion only, as no approach really ever makes sense when working with implausible data, but lets see what more you find.

sgorm123 commented 2 years ago

We (Catherine, @ljin22 , and me) have spoken to a couple of our Global Statistical Leads and our Standards Group, who are quite clear this is a data issue (as we conveyed previously) and we would not accept this issue if it remained for any external deliverable.    However, if this data issue should remain (for some reason) for internal reporting purposes (i.e. not a regulatory submission) then the product teams have two options:

  1. Implement this rule:

    • If a CR is observed at the first time point, then any disease seen at subsequent time points, even disease meeting PR criteria relative to baseline, makes the disease PD at that point (since disease must have reappeared after CR). • Then in this case, for Amgen, the best confirmed response would be SD (caveated again by the fact that this is actually a data issue, and not supported globally by our standards team). • We would then exclude assessments after the initial PD for the derivation of confirmed overall response (i.e. removed from the input dataset being passed into the derivation)

Catherine's thoughts on this (and not Amgen’s practices), are that best confirmed response in this case should be a PR given that the PD is programmed and not on the raw data.  

  1. Do nothing, so it remains CR PR PR PR and the best confirmed response would be PR. 

Note: In both cases above a footnote would be added to clearly show this data issue.   FYI - Amgen rules for unconfirmed response, are much more complex than the Roche specs (which I used to program the bor unconfirmed function). We also have many PARAM'S with various options for unconfirmed response, so if we did observe this data then the bor would depend on the PARAM's definitions.

Please also note, the product teams do have some autonomy on this decision, so it could be different across products.  Therefore (and as this did take us a little while to research and understand, with even more flavours on this algorithm) we agree with your comment @rossfarrugia, to just add the assertion for now and consider the additional rules later.

Thanks for your patience and do let us know if you have any questions on any of the above @rossfarrugia @bundfussr @amitjaingsk. Have a lovely weekend, Stephen and Catherine