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

updates to noresm_develop to be close as possible to noresm2_3_develop #13

Conversation

mvertens
Copy link
Contributor

@mvertens mvertens commented Jan 28, 2024

This PR updates the oslo_aero code base so that there are minimal differences between the branches noresm_develop and noresm2_3_develop.

The only files that are different between the two in src/ are:

oslo_aero_control.F90 
oslo_aero_nucleate_ice.F90 
oslo_aero_optical_params.F90 

This PR also resolved Issue #12 which now passes.

mvertens added a commit to mvertens/CAM that referenced this pull request Jan 28, 2024
@oyvindseland
Copy link

Does this mean that the SE dycore also works in a 2.3 setting with Oslo-Aero or is it the development version pointing towards 2.5?

@mvertens
Copy link
Contributor Author

@oyvindseland - there are two development branches of oslo-aero. One that goes into 2.3 and one that goes into 2.5. The actual oslo-aero code (in src/) is almost the same between the two except for one file with major differences. The shadow files (in src_cam/) are somewhat different - which is to be expected. You should be able to run the spectral element in 2.3 - but its not officially supported in that version. So @gold2718 will be extending the noresm2_3_develop branch of oslo-aero for the noresm2.3 release and I will be working on the noresm2_5_develop version for noresm2.5. Does this help clarify things?

@oyvindseland
Copy link

OK. Good to know

Copy link
Contributor

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Looks good. Just a couple of comments about future cleanup

Comment on lines +642 to +643
( (l_index_donor == chemistryIndex(l_so4_a1)) .and. &
modeIndexCoagulator == MODE_IDX_OMBC_INTMIX_COAT_AIT) ) then
Copy link
Contributor

Choose a reason for hiding this comment

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

There are parentheses around the first equality check but not the second one. Something to fix someday?

Comment on lines +668 to +670
if( .NOT. is_process_mode(l_index_donor, .true.) .or. &
( (l_index_donor == chemistryIndex(l_so4_a1)) .and. &
coagulatingMode(iCoagulator) == MODE_IDX_OMBC_INTMIX_COAT_AIT) ) then
Copy link
Contributor

Choose a reason for hiding this comment

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

There are parentheses around the first equality check but not the second one. Something to fix someday?

@mvertens
Copy link
Contributor Author

@gold2718 - so I'm holding off a bit on this PR since the threading seems to be broken (ERP test crashes). I'm trying to track down if its the oslo_aero code or not. The PEM test passes. I'll raise issues once I sort this out.

@mvertens mvertens changed the title updates of code to be as close as possible to noresm2_3_develop updates to noresm_develop to be close as possible to noresm2_3_develop Jan 30, 2024
Copy link
Contributor

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Would appreciate it if you would re-introduce indentation to radiation.F90 but otherwise looks good.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the old indentation even though that is not (yet?) used in the NorESM2.3 code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay - I will reintroduce it.

@gold2718 gold2718 added the enhancement New feature or request label Jan 30, 2024
@gold2718 gold2718 self-assigned this Feb 20, 2024
@gold2718 gold2718 merged commit 8a3ff7d into NorESMhub:noresm_develop Feb 20, 2024
@gold2718 gold2718 added this to the NorESM2.5 milestone Feb 20, 2024
@mvertens mvertens deleted the feature/update_noresm_develop_to_noresm2_3_develop branch February 20, 2024 11:39
gold2718 added a commit to NorESMhub/CAM-Nor-physics that referenced this pull request Feb 21, 2024
…noresm2_3_develop

Updates for changes to oslo aero to bring in noresm2_3_develop changes

Updates that must accompany changes to oslo_aero
Summary:Updates that must accompany changes to oslo_aero
Branch: this will not be tagged but stay on the noresm branch
Contributors: mvertens
Reviewers: @gold2718

Purpose of changes: updates needed to support changes in oslo_aero PR NorESMhub/OSLO_AERO#13

Github PR URL: #8
Changes made to build system: None
Changes made to the namelist: None
Changes to the defaults for the boundary datasets: None
Substantial timing or memory changes: None
Testing: Verified that the following tests passed:
- ERP_Ln9.ne30pg3_ne30pg3_mtn14.NF1850oslo.betzy_intel.cam-outfrq9s
Issues addressed by this PR: #9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants