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

TOX-534 Check acy() usage for concentration est in tcplfit2_core & tcplhit2_core #3

Open
brown-jason opened this issue May 2, 2023 · 4 comments
Assignees

Comments

@brown-jason
Copy link
Collaborator

tcplfit2_core uses acy to estimate a theoretical top and the AC50 (gain and loss, if applicable) for curve fits.

tcplhit2_core uses acy to estimate other concentration estimates (e.g. AC5, AC10, BMD, etc.).

To Do:

Investigate if there is a reason for the AC50 being calculated in tcplfit2_core rather than with the other concentration estimates.
If yes, then provide additional documentation within the code to provide an quick guidance (bread crumb) explanation. For example, if the AC50 is part of the necessary model parameters used downstream.
If no, then document and may consider creating a ticket to "house" all of the concentration estimations together under the same function (i.e. only need to use acy() for tcplhit2_core rather than in both places).

@sedavid01
Copy link
Collaborator

Assigning to Grace.

@gracezhihuizhao
Copy link
Contributor

What the code is doing now is that we will pass the result of tcplfit2_core() to tcplhit2_core(), and in tcplhit2_core() it calls hitloginner() to calculate discrete hit. hitloginner() then takes AC50 as an input, so AC50 has to be in the result that being passed in (other estimators are calculated after hit logic).

However, maybe hitloginner() was updated, but AC50 is marked as no longer unnecessary for this function now, so I believe it doesn't has to be calculated in tcplfit2_core. I'm not sure if this is intended, please let me know.

@sedavid01
Copy link
Collaborator

@brown-jason , are there any further actions/tickets that need to be made from this information collection?

@sedavid01
Copy link
Collaborator

@brown-jason preference is to leave as-is for now. We can always revisit it if there are bugs that crop up and/or necessity to 'relocate' this calculation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants