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

Make ozone photochem diagnostics match parametrization terms #196

Closed
wants to merge 1 commit into from

Conversation

DWesl
Copy link

@DWesl DWesl commented Apr 9, 2024

The ozone photochemistry parametrization is a first-order Taylor expansion of the net ozone production in three variables. This commit changes the four diagnostics the parametrization manages to match the four terms in the Taylor expansion.

When comparing to the implementation in lines 301-304, note that it is implicit in the model ozone mixing ratio, so line 301 has the first term and the second half of the second term, while the first half of the second term is part of the denominator on line 304.

I could use ozi or ozib for an explicit approximation to that term, but we already calculated the value needed for the implicit calculation actually used for the update.

I was told to open the PR here rather than the NCAR repository (NCAR#1068)

The ozone photochemistry parametrization is a first-order Taylor expansion of the net ozone production in three variables.  This commit changes the four diagnostics the parametrization manages to match the four terms in the Taylor expansion.

When comparing to the implementation in lines 301-304, note that it is implicit in the model ozone mixing ratio, so line 301 has the first term and the second half of the second term, while the first half of the second term is part of the denominator on line 304.

I could use ozi or ozib for an explicit approximation to that term, but we already calculated the value needed for the implicit calculation actually used for the update.
@dustinswales
Copy link
Collaborator

@DWesl I think these changes are fine.
But just to be sure, if I understand correctly, we were computing diagnostics for internal calculations, not the true quantities in the expansion represented by equation 4 in the manuscript?
I'm not sure who uses these diagnostics in the UFS, so we should see if this will have an unforeseen impact someones work.
@yangfanglin @Qingfu-Liu Thoughts?

@DWesl
Copy link
Author

DWesl commented Apr 16, 2024

@DWesl I think these changes are fine. But just to be sure, if I understand correctly, we were computing diagnostics for internal calculations, not the true quantities in the expansion represented by equation 4 in the manuscript?

For the first diagnostic, yes: it looks like the first line of the tendency code.

The second diagnostic would likely have ended up a copy of dtend_o3_photochem if that diagnostic is accumulated elsewhere.

@grantfirl
Copy link
Collaborator

@DWesl Is there a reason that this is still in draft mode? Do you need help running UFS regression tests?

@DWesl
Copy link
Author

DWesl commented Apr 19, 2024

I hadn't entirely realized I could run the regression tests myself. I was waiting until I could recompile and check whether the new diagnostics made sense. I have data now, and should have some idea in the next few hours.

@dustinswales
Copy link
Collaborator

@DWesl After some thinking I'm not sure we need to make this change.
With this change, the diagnostics will be consistent with the published results, but inconsistent with how it's formulated above.
If so, which is more valuable?

@DWesl
Copy link
Author

DWesl commented Apr 19, 2024

With this change, the diagnostics will be consistent with the published results, but inconsistent with how it's formulated above. If so, which is more valuable?

Could you clarify what you mean by "inconsistent with how it's formulated above"?

The second diagnostic doesn't show up explicitly in the implementation:

do3_dt_ozmx(:,iLev) = (oz(:,iLev) - ozib(:))

As mentioned previously, I suspect this would end up being a copy of dtend_o3_photochem if that is calculated in whatever function calls run_o3prog_2015

The first diagnostic excludes the rest of the implicit update for ozone:

oz(iCol,iLev) = (ozib(iCol) + tem*dt) / (1.0 - prod(iCol,2)*dt)

if you really want to match the diagnostics to the lines of the implementation, having the second diagnostic match this line, or perhaps just the denominator of this line, might work better?

Personally, I find it easier to interpret diagnostics that match the result of the implementation rather than the details, which is related to why I used oz rather than ozi or ozib in the second diagnostic.

@DWesl
Copy link
Author

DWesl commented Apr 30, 2024

In other words,
$$(P-L)(t) \approx (P-L)|_0 + \left.\frac{\partial(P-L)}{\partial r}\right|_0 (r(t + dt) - r_0) + \cdots$$
$$\frac{dr}{dt}(t) \approx (P-L)_0 + \left.\frac{\partial(P-L)}{\partial r}\right|_0 r(t + dt) - \left.\frac{\partial(P-L)}{\partial r}\right|_0 r_0 + \cdots$$
$$r(t + dt) - r(t) \approx (P-L)_0 dt + \left.\frac{\partial(P-L)}{\partial r}\right|_0 r(t + dt) dt - \left.\frac{\partial(P-L)}{\partial r}\right|_0 r_0 dt + \cdots$$
$$r(t + dt) - \left.\frac{\partial(P-L)}{\partial r}\right|_0 r(t + dt) dt \approx r(t) + (P-L)_0 dt - \left.\frac{\partial(P-L)}{\partial r}\right|_0 r_0 dt + \cdots$$
$$r(t + dt) \left(1 - \left.\frac{\partial(P-L)}{\partial r}\right|_0 dt\right) \approx r(t) + dt \left((P-L)_0 - \left.\frac{\partial(P-L)}{\partial r}\right|_0 r_0 + \cdots\right)$$
$$r(t + dt) \approx \frac{r(t) + dt \left((P-L)_0 - \left.\frac{\partial(P-L)}{\partial r}\right|_0 r_0 + \cdots\right)}{1 - \left.\frac{\partial(P-L)}{\partial r}\right|_0 dt}$$
I prefer to use the first line rather than the last for the diagnostics.

Plots of these diagnostics look sensible, and reflect changes in the parametrization.

@DWesl DWesl marked this pull request as ready for review April 30, 2024 21:08
@grantfirl
Copy link
Collaborator

@DWesl Since this is a relatively minor change, I'd like to combine it with others for purposes of the UFS merge queue. My understanding is that it should change regression test results for any tests that have diagnostics activated and that output the diagnostic that you're changing.

Are you OK with me combining this with other PRs?

@DWesl
Copy link
Author

DWesl commented May 3, 2024

My understanding is that it should change regression test results for any tests that have diagnostics activated and that output the diagnostic that you're changing.

Yes, that matches my understanding.

Are you OK with me combining this with other PRs?

Yes

@grantfirl
Copy link
Collaborator

For future reference: changes baselines for tests that use diag_additional_control_dtend or diag_additional_rap_dtend diag tables: control_diag_debug_intel/gnu, rap_diag_debug_intel/gnu

@grantfirl
Copy link
Collaborator

Combined into #205

@grantfirl grantfirl closed this May 15, 2024
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

Successfully merging this pull request may close these issues.

None yet

4 participants