rrsg2020 / analysis

4 stars 0 forks source link

NIST Temperature correction: feature only available for SN >= 42 & documentation #26

Open mathieuboudreau opened 3 years ago

mathieuboudreau commented 3 years ago

Hi @jvelazquez-reyes ,

At a previous meeting together (or maybe in an email thread), we had discussed how to handle the temperature correction for serial numbers < 42. Katy Keenan had mentioned that their recommendation from NIST was to apply the fits for the temperature correction of serial numbers >= 42 to those of less than 42 (note that this is applying the polynomials of the fits from one to the other, not replacing the T1 values of one for the other). Below is here email:

Hi Mathieu,

Our (NIST) recommendation is to apply the fits from the data for SN >= 42 to the phantoms with SN < 42. While the measured T1, T2 values are different between these two categories, the temperature dependence should not be different. 

We do recommend fitting a log-log representation of the data using a low-order polynomial.

If you have any additional questions as you try to implement it, feel free to ask.

Thank you,
Katy

I thought this was done, but just noticed that the tool is only working for serial numbers >42 at the moment (see here: https://github.com/rrsg2020/analysis/blob/289e4c4c4dd58816a0636ae43df7c89b086dbac6/src/nist.py#L73). Is this something that was done in a branch but never merged? If not, do you think it would be possible to do soon?

Also, it would be helpful if that function had some documentation at the top of the function (e.g. describing all the arguments, what it returns, and some sample usage). Katy asked if the function was available for sharing with other groups, so this would be useful information for them.

Thanks!

jvelazquez-reyes commented 3 years ago

@mathieuboudreau please help me understand this:

note that this is applying the polynomials of the fits from one to the other, not replacing the T1 values of one for the other

In the code, we have a dictionary with 14 spheres, each one with reference T1 values at 16, 18, 20, 22, 24, and 26°C. If I understand well, these reference values correspond to phantom serial numbers >= 42 (see here):

https://github.com/rrsg2020/analysis/blob/5e1e934e2c6fe9f896c220b66dc5110e13ba381a/src/nist.py#L75-L90

Based on this dictionary, we use the interp1d function to obtain what we are calling the polynomial fits. In this case, the polynomial fits are stored on the variable quad for the quadratic interpolation (see here):

https://github.com/rrsg2020/analysis/blob/5e1e934e2c6fe9f896c220b66dc5110e13ba381a/src/nist.py#L134

Then, when we call quad, we want that whether the input serial number is >= 42, or < 42, we will apply the polynomial fits that come from the data for phantom serial numbers >= 42.

Is this correct? If so, I don't understand what do you mean by replacing the T1 values of one for the other.

mathieuboudreau commented 3 years ago

Hi Juan,

Sorry for the confusion. It's that the fits for >= 42 will be relative to the references T1 values (e.g. at 20 degrees C) of that version of the phantom (for serial numbers >=42). So in the case of serial numbers <42, we can make the assumption that the T1 will vary with temperature along the same curve as for >=42, but with a difference reference T1 value (e.g. at 20C). So I think for serial numbers of <42, we could just apply the fit as you're using already for >=42, but renormalize it to the reference T1 value for version 1 of t he phantom (serial numbers 42).

Basically, I think if you apply the current algorithm as is for a measurement on a serial number <42, we would just need to divide the resulting T1 value by T1(SN>=42 at 20C) and then multiply by T1(SN<42 at 20C). You can verify if this would work for the case of temperature at exactly 20C for a SN<042; in this case, the outputed value should simply be T1(SN<42 at 20C) after the "fit" due to the renomarlization I explain here.

If this is still confusing (which it might be, I'm typing this very late at night), then we could work through the steps (and write tests for it, which we should anyways) during a video call.

Best,

Mathieu

jvelazquez-reyes commented 3 years ago

@mathieuboudreau temperature correction is now available for phantom serial numbers <42. I created a pull request, but maybe we don't want to merge all commits ahead from the branch statistical_analysis_R into master yet.

mathieuboudreau commented 3 years ago

Hi @jvelazquez-reyes,

Was this merged? I can't recall. If so, please close this issue

jvelazquez-reyes commented 3 years ago

Hi @mathieuboudreau ,

I created the branch temperature_correction to implement this feature but it has not been merged. I opened a PR and you did a first review. I addressed the changes you requested but I think you haven't reviewed them. You can see here

mathieuboudreau commented 3 years ago

Ah ok perfect – thanks for catching me up to speed!

I had recalled that we had pushed some changes related to to NIST temp in some repo for Katy, but that might have been a separate repo – do you recall where (and is this PR up to date with those changes)? Sorry for the confusion, I'll get myself back up to speed with this PR.

jvelazquez-reyes commented 3 years ago

Initially, I had pushed these changes to the existing branch statistical_analysis_R but working on that branch was not convenient. So, you suggested opening and pushing these changes to a new one, and I created temperature_correction. This PR is up to date with those changes for Katy.