Skip to content
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

Closed
wants to merge 4 commits into from

Conversation

kartikeyakirar
Copy link
Contributor

@kartikeyakirar kartikeyakirar commented Jun 12, 2024

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 earlier LOQFL_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:

  • Fix ajax error on selecting different biomarker DT table throws ajax error

@kartikeyakirar kartikeyakirar marked this pull request as draft June 12, 2024 19:50
@npaszty
Copy link
Contributor

npaszty commented Jun 20, 2024

@kartikeyakirar

didn't realize you requested my review on this. I can't test these updates easily so can @gogonzo review this instead? thanks

Comment on lines 686 to 697
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())]]))
)
Copy link
Contributor

@gogonzo gogonzo Jun 23, 2024

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 vs LBSTRESC checkup? I thought checking AVAL vs LBSTRESC is just a backup procedure if LOQFL has NA or "".

Copy link
Contributor

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

Copy link
Contributor

@gogonzo gogonzo Jun 25, 2024

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"
)

Copy link
Contributor

@gogonzo gogonzo left a 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

gogonzo added a commit that referenced this pull request Jun 27, 2024
alternative to
#280

Simplified the code by replacing pivot_ calls with simple filter and
select.
@gogonzo gogonzo closed this Jul 3, 2024
@gogonzo gogonzo deleted the fixing_correlation_calcualtion branch July 3, 2024 08:28
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants