Skip to content

Conversation

@jaelynlitz
Copy link
Collaborator

The bulk of the port is there - some resulting questions though:

@jaelynlitz jaelynlitz force-pushed the jaelynlitz/mo_sethet_dev branch from 9336444 to defd7b9 Compare March 9, 2024 01:14
@codecov
Copy link

codecov bot commented Mar 9, 2024

Codecov Report

Attention: Patch coverage is 89.65517% with 15 lines in your changes missing coverage. Please review.

Project coverage is 96.07%. Comparing base (636ab95) to head (4ddf27f).
Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
src/mam4xx/mo_sethet.hpp 89.65% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #295      +/-   ##
==========================================
- Coverage   96.16%   96.07%   -0.09%     
==========================================
  Files          43       44       +1     
  Lines        9486     9651     +165     
==========================================
+ Hits         9122     9272     +150     
- Misses        364      379      +15     
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jaelynlitz jaelynlitz force-pushed the jaelynlitz/mo_sethet_dev branch from defd7b9 to d782df2 Compare March 30, 2024 00:06
@jaelynlitz jaelynlitz force-pushed the jaelynlitz/mo_sethet_dev branch from a267a1c to ad27d8d Compare April 24, 2024 23:02
@jaelynlitz
Copy link
Collaborator Author

This is finally ready to go! With help from @odiazib and @mjs271 yesterday - we tracked down the last of the indexing issues.

The outstanding discrepancy is around ktop. In the fortran, they compute the min ktop value across the 4 columns to optimize/group their operations. Because the C++ is a single column model, we just compute ktop for our column and operate from there. This results in a few values being computed in the C++ that the fortran skips. I have added the C++ computed values to the fortran validation data to allow for this difference while keeping a high tolerance on our tests.

Copy link
Collaborator

@jeff-cohere jeff-cohere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me! I've left a couple of minor comments. Is there anything specific that you'd like a closer look at?

@jaelynlitz jaelynlitz force-pushed the jaelynlitz/mo_sethet_dev branch from ad27d8d to f006392 Compare August 13, 2024 21:57
@jaelynlitz jaelynlitz self-assigned this Aug 13, 2024
@jaelynlitz
Copy link
Collaborator Author

I removed the Kokkos for loop in gas_washout and that seemed to fix the precision issues I was seeing - let me know if we want to look into putting that back. Please forgive the embarrassingly long lead time on this - ready for final review @jeff-cohere @mjs271 @odiazib @singhbalwinder @overfelt

Copy link
Contributor

@singhbalwinder singhbalwinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work! Thanks! I have some minor comments but overall looks good to me.

precip(kk) = cmfdqr(kk) + nrain(kk) - nevapr(kk);
});

for (int kk = 0; kk < pver; kk++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to change this for loop to a Kokkos::parallel_for.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use Kokkos::ThreadVectorRange.

});
team.team_barrier();

for (int kk = ktop; kk < pver; kk++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kokkos::parallel_for here.

// ... part 2, in-cloud solve for low henry constant
// hno3 and h2o2 have both in and under cloud
//-----------------------------------------------------------------
for (int kk = ktop; kk < pver; kk++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kokkos::parallel_for.


for (int mm = 0; mm < gas_wetdep_cnt; mm++) {
int mm2 = wetdep_map[mm];
for (int kk = 0; kk < ktop; kk++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kokkos::parallel_for.

@jaelynlitz jaelynlitz force-pushed the jaelynlitz/mo_sethet_dev branch from 06f50fa to cc24f32 Compare September 27, 2024 23:47
@jaelynlitz
Copy link
Collaborator Author

a few more parallel_fors to add and the passing tolerance for sethet is 1e-3 which seems a little too big

@jaelynlitz jaelynlitz force-pushed the jaelynlitz/mo_sethet_dev branch from 4ddf27f to 31f3869 Compare October 28, 2024 21:49
@jaelynlitz jaelynlitz force-pushed the jaelynlitz/mo_sethet_dev branch from 17669e8 to d4099e7 Compare November 6, 2024 22:34
@jaelynlitz
Copy link
Collaborator Author

close in favor of #373

@jaelynlitz jaelynlitz closed this Nov 8, 2024
@jaelynlitz jaelynlitz mentioned this pull request Nov 8, 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.

6 participants