-
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing LOQFL_COMB
calculation
#280
Conversation
didn't realize you requested my review on this. I can't test these updates easily so can @gogonzo review this instead? thanks |
R/tm_g_gh_correlationplot.R
Outdated
x_temp = ifelse( | ||
.data[[.(xloqfl())]] == "Y" & ( | ||
(grepl("<", .data[[.(xlabs())]]) & .data[[.(xvar())]] < as.numeric(gsub("[^0-9.-]", "", .data[[.(xlabs())]]))) | | ||
(grepl(">", .data[[.(xlabs())]]) & .data[[.(xvar())]] > as.numeric(gsub("[^0-9.-]", "", .data[[.(xlabs())]]))) | ||
), "Y", ifelse(.data[[.(xloqfl())]] == "Y", "N", as.character(.data[[.(xloqfl())]])) | ||
), | ||
y_temp = ifelse( | ||
.data[[.(yloqfl())]] == "Y" & ( | ||
(grepl("<", .data[[.(ylabs())]]) & .data[[.(yvar())]] < as.numeric(gsub("[^0-9.-]", "", .data[[.(ylabs())]]))) | | ||
(grepl(">", .data[[.(ylabs())]]) & .data[[.(yvar())]] > as.numeric(gsub("[^0-9.-]", "", .data[[.(ylabs())]]))) | ||
), "Y", ifelse(.data[[.(yloqfl())]] == "Y", "N", as.character(.data[[.(yloqfl())]])) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please name these temporary vars x/yloqfl_temp
. Please replace ifelse
with case_when
.
question to @npaszty :
- Do we need to confirm when "LOQFL" is "Y" or "N" with
AVAL
vsLBSTRESC
checkup? I thought checking AVAL vs LBSTRESC is just a backup procedure if LOQFL hasNA
or "".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to chat more offline but hope this helps.
LBSTRESC, a standard SDTM variable in the SDTM LB domain, is a character class variable so its values can contain LoQ information in the form of "<" or ">". Other symbols can also be present in the values but we focus on the "<" or ">". So LBSTRESC should be thought of as the source variable in LB from which AVAL values, a standard ADaM variable in ADLB for example, are assigned/derived. AVAL is a numeric class variable so can not contain symbols like "<" or ">" and in fact is NA if LBSTRESC value does contain anything other than just a numeric value.
For LoQ values, we derive AVAL value from LBSTRESC when LBSTRESC contains "<" or ">" symbols. If we do this then we set LOQFL to "Y" for AVAL. @gogonzo you are correct that this introduces a design flaw specific to the correlation plot where combinations of PARAMCD and Analysis variable together will display incorrect LoQ information.
The analysis variables that can be currently selected are...
- AVAL: The analysis value at any given visit. LoQ relevant.
- BASE: The baseline analysis value which is carried forward across all visits within a subject's records within a PARAMCD. LoQ relevant.
- CHG: Change from baseline at any given visit within a subjetc's records within a PRAMCD. Not LoQ relevant.
- PCHG: Percent change from baseline at any given visit within a subjetc's records within a PRAMCD. Not LoQ relevant.
- AVALL2: Log 2 of AVAL. Not LoQ relevant.
So currently the only variables where LoQ concept is relevant are AVAL and BASE. So we could add LOQFLB (I would add this to ADLB so the app would expect it was present) and then include that in the way the module processes the data. I think this would work if AVAL + BASE are selected as the analysis variables and also work if AVAL + CHG or BASE + CHG etc. were selected.
I suppose in the module these could be assigned as LOQFLyaxis and LOQFLxaxis to make the names more generic going through the pivoting data process in the module. But I will leave that part up to you guys.
Hope the table illustrates the expected outcome though I'm not sure what the best design is for the module so that it reflects this in the brushed table and also with the correct symbols in the plot.
Analysis Variable x | Analysis Variable y | LOQFL | LOQFLB | LOQFL_COMB |
---|---|---|---|---|
AVAL | BASE | N | N | N |
AVAL | BASE | N | Y | Y |
AVAL | BASE | Y | N | Y |
AVAL | CHG | Y | not relevant for CHG | Y |
AVAL | PCHG | N | not relevant for PCHG | N |
AVALL2 | AVAL | not relevant for AVALL2 | N | N |
AVALL2 | AVAL | not relevant for AVALL2 | Y | Y |
AVAL | BASE | Y | Y | Y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kartikeyakirar I think above can be simplified with a condition:
LOQFLB = <LOQFL at the Baseline repeated for all vistits>,
LOQFL_COMB = case_when(
"AVAL" %in% c(xvar(), yvar()) & LOQFL == "Y" ~ "Y",
"BASE" %in% c(xvar(), yvar()) & LOQFLB == "Y" ~ "Y",
TRUE ~ "N"
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, it is a problem of a wrong preprocessing and miss-flagging LOQFL
. Please look at the THEORY/preprocess.R#111
I think fixes should be made there.
Why do I think this way?
- app interactivity doesn't influence LBSTRESC nor LOQFL values.
- if LOQFL and LBSTRESC remains constant they should be consistent when passed to the app
alternative to #280 Simplified the code by replacing pivot_ calls with simple filter and select.
This PR fixes the calculation of
LOQFL_COMB
:To check this, access the
THEORY
app.In the correlation plot, there are controls for Biomarker (PARAM) and Analysis Variable (AVAL, BASE, etc.) for each axis. For the LoQ legend to display "Y", one of the PARAM values must have LOQFL == "Y". Albumin does not have any LOQFL == "Y" values, and it is the PARAM that has been selected for both axes.
PR add calcaltion for
LOQFL_COMB
earlierLOQFL_COMB
is "Y" when neither of those values is LOQFL == "Y" in the data.For example, LOQFL_COMB is "Y" for BASE.ALT x AVAL.ALBUM at week 12 when neither is a LoQ value.
TODO: