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

Refactor to modify x2l instead of atm2lnd #40

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

mvhulten
Copy link
Collaborator

Here x2l is updated by ICON variables instead of atm2lnd.

It is strictly not a pure refactoring of the master code, because the output is different. However, the output from refac-x2l-incr is identical (in any output variables for any timestep) to that in master+vars+patch. The branch master+vars+patch is a (somewhat messy) bug fix to master without any refactoring. It is only meant as a proof of concept to show that refac-x2l-incr is good to merge.

For this to work, it was needed to
- Switch oas_receive_icon() and lnd_import()
- Store rain in "large scale" rainl, set "convective" rainc to zero
- Store snow in "large scale" snowl, set "convective" snowc to zero
- Removed redundant assignment to atm2lnd_inst%forc_solar_grc(:)
- Enabled it again in case ICON in clm5/cpl/lnd_import_export.F90
- Enabled checks again for ICON in clm5/cpl/lnd_import_export.F90
The precompiled code from 'refac-x2l-incr' is now identical to the precompiled
code from 'master'.
@mvhulten mvhulten requested a review from s-poll July 12, 2024 14:59
Copy link
Member

@s-poll s-poll left a comment

Choose a reason for hiding this comment

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

As discussed earlier, the PR shows an improvement to the src. I think the PR is ready to be merged, when the ifdef-statement in src/clm5/cpl/lnd_import_export.F90 has been clarified.

src/clm5/cpl/lnd_import_export.F90 Show resolved Hide resolved
@s-poll
Copy link
Member

s-poll commented Jul 15, 2024

@AGonzalezNicolas @kvrigor Can one of you merge the PR? Or are we allowed to merge it by our own?

@kvrigor kvrigor merged commit b882524 into HPSCTerrSys:master Jul 15, 2024
3 checks passed
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.

3 participants