-
Notifications
You must be signed in to change notification settings - Fork 16
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
308 add mc5 method loec.coff #312
Conversation
@Kelly-Carstens-EPA please test with your new dataset (method should be available in invitrodb or invitrodb_test; method id 15) Suggested text for mc5_mthds description: "Identify the lowest observed effective concentration (loec) where the absolute value of all responses fall above cutoff. If loec exists, assume hit call = 1, fitc = 100, model_type = 1. Winning model is not selected based on curve fits and therefore additional potency estimates are not derived." Suggested text for model_type LOEC description: "Data is fit bidirectionally, but winning model is not selected and therefore additional potency estimates are not derived. Instead the lowest observed effective concentration (loec) where the absolute value of all responses fall above cutoff is identified, and if loec exists, hit call is assumed active." @sedavid01 thoughts on this description? |
@madison-feshuk and @cthunes, I think the suggested descriptions are overall representative of the updated method applied. Only suggestion I would consider, but you can ignore as you best see fit, would be to describe it as "value of all responses are outside the cutoff band (i.e. abs(resp) > cutoff)". I think this is more intuitive since data is being fit bidirectionally. Current way things are written is accurate, but could be confused by some (if not read carefully) that we only look for responses in the positive direction. Happy to discuss further if there are additional questions, comments, etc. |
I like this and feel the same. Will incorporate and move forward with that and happy to update again if needed. Thanks @sedavid01! |
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.
Assuming the code, when tested, works as intended for the outlined behavior desired for the LOEC method, the code and description seems to be appropriate. Aside from some methodological questions, which are related but slightly out of scope for this ticket (i.e. differential behavior for determining the LOEC) I have no further blockers on the implementation of this method.
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.
I pipelined new data into invitrodb_test (aeid: 3298) and successfully pipelined applying the new mc5 method 15. The hit calls and loec's look good. I noticed the tcplPlot() function is broken, which is not surprising. I think it could still be helpful to see plots similar to how we view 'sc' data. I will go ahead and approve this PR, assuming this would be a separate ticket. Thx!
Plotting error:
tcplPlot(type = "mc", fld = "m4id", val = mc5$m4id, output = "pdf", multi = TRUE, verbose = TRUE, fileprefix = "Neurosetta_mc5_30perc_PR_loec_06dec24")
Error in scale_x_continuous()
at CompTox-ToxCast-tcpl/R/tcplPlot.R:988:5:
! Discrete values supplied to continuous scale.
ℹ Example values: NA, NA, NA, NA, and NA
Run rlang::last_trace()
to see where the error occurred.
There were 50 or more warnings (use warnings() to see the first 50)
Thanks for testing Kelly, glad everything looks good. That behavior is 100% expected from tcplPlot. It's looking for winning model parameters which don't exist. #311 is my current working ticket and I will let you know when you could test it out! |
Added loec method and updated mc5 accordingly. Keep in mind that some portion of level 6 flags will not work anymore since we don't have winning model parameters. Closes #308
Still to be completed: