ggebbie / HistoricalIndianOcean

Historical Indian Ocean Temperature Change
MIT License
0 stars 0 forks source link

Changes in Tratio and Sratio #1

Closed wenegrat closed 2 years ago

wenegrat commented 2 years ago

Thought I'd document some questions here, rather than via email:)

In the last commit (b41d2a4) there were some changes in the calculation and parameters that led to changes in the profiles.

1) First I see Sratio was changed to S_ratio = (1/5) rather than S_ratio = (1/5)^2. I assume this was just a mistake in the prior version.

2) The calculation to Rtt has been updated to reflect our conversation about how Tratio should be used. I have a question about how this is currently calculated though. We realized that Rtt = (Tratio + 1)*Rnn. In the code you have:

Rtt = weighted_covariance(locs,ση,LxyT,LzT) + weighted_covariance(locs,√tratio*ση,LxyT,LzT)

This though seems somewhat different than above, as √(1+tratio) != 1 + √tratio. Doing it the way it is currently in the code increases the factor from 2 (with tratio=3) to 2.7. You have a much better understanding of this code and method, but I would have thought we want to increase Rnn by a multiplicative factor, so we could pose this as,

Rtt = √(1+tratio)*weighted_covariance(locs,ση,LxyT,LzT)

ggebbie commented 2 years ago
  1. To make things more consistent, I wanted Sratio and Tratio to both refer to a fraction of variance. Previously, Sratio was defined as a "magnitude" or standard deviation and so I updated it.

  2. It turns out that the line above in your comment was ok. If it is replaced with this line Rtt = (tratio + 1) * weighted_covariance(locs,ση,LxyT,LzT) it gives the same numerical result. I've updated the code with this new line that is easier to read and is consistent with the docs.

I was able to reproduce the results used in the manscript in commit d2a500ada0bf3f46d542248e12eb2a67a4fea03a. These results originally come from commit 58b1f0ab9900b2a13e128417b7df1806ebfac0df.

There are just two differences to the most recent version of the code and they are related to calculating Rss. The differences can be seen in commit 9e04fbce10a6adb87d483fe58aaf66238294f479. I don't understand why the previous Rss was using the wrong sratio and lengthscales. Both of these numerical changes are necessary to reconstruct the previous results.

wenegrat commented 2 years ago

2. It turns out that the line above in your comment was ok. If it is replaced with this line Rtt = (tratio + 1) * weighted_covariance(locs,ση,LxyT,LzT) it gives the same numerical result. I've updated the code with this new line that is easier to read and is consistent with the docs.

Agreed, my formulation was incorrect.

There are just two differences to the most recent version of the code and they are related to calculating Rss. The differences can be seen in commit 9e04fbc. I don't understand why the previous Rss was using the wrong sratio and lengthscales. Both of these numerical changes are necessary to reconstruct the previous results.

OK, to confirm my understanding, the version in the draft manuscript (as of right now) is incorrect because sratio and lengthscales were incorrect. However, the newest commit 9e04fbc fixes this (and also incorporates the changes to how tratio is used)? If so, can you please re-run the analysis and I'll update figures.

ggebbie commented 2 years ago

Correct, both sratio and lengthscales in Rss were inconsistent with the supporting info.

Yes, commit 9e04bc fixes this and also incorporates the simple equation for Rtt that involves Tratio.

Because Rss was the problem, it's best to be explicit about how I computed it in the supporting info. Commit 7ff978de1aa774ee0c334ac58dca642d232fbf41 doesn't change anything in the calculation but updates the documentation.

Next I will re-run and send to Google Drive.

wenegrat commented 2 years ago

Closing issue as per @ggebbie comment above. Manuscript is updated to use the corrected results.