E3SM-Project / v3atm

Fork of E3SM for testing v3 atm changes
Other
0 stars 5 forks source link

Explicit SOA + chemUCI+Linozv3 branch based on NGD_v3atm fork (Branch: mkshriva/atm/chemUCI_SOA_Merged_rebase1) #6

Closed quaz115 closed 1 year ago

quaz115 commented 1 year ago

This PR is for dynamic (Explicit) SOA updates on NGD_v3atm fork . The branch mingxuanwupnnl/atm/chemUCI-chemMZT-MOSAIC_AMIP (*CORRECTION) was used as a base for this current branch with updates on Explicit SOA

Summary of evaluation done for v2 with the Explicit SOA updates can be found at: https://acme-climate.atlassian.net/wiki/spaces/NGDAP/pages/3437166635/New+Secondary+Organic+Aerosol+SOA+features+for+E3SM+v3

Relevant code modifications to add Explicit SOA with chemUCI+Linozv3: modified: components/eam/bld/build-namelist modified: components/eam/src/chemistry/modal_aero/aero_model.F90 modified: components/eam/src/chemistry/modal_aero/modal_aero_amicphys.F90 modified: components/eam/src/chemistry/modal_aero/modal_aero_data.F90 modified: components/eam/src/chemistry/modal_aero/modal_aero_initialize_data.F90 modified: components/eam/src/chemistry/modal_aero/module_data_mosaic_aero.F90 modified: components/eam/src/chemistry/modal_aero/module_mosaic_astem.F90 modified: components/eam/src/chemistry/modal_aero/module_mosaic_box_aerchem.F90 modified: components/eam/src/chemistry/modal_aero/module_mosaic_cam_init.F90 modified: components/eam/src/chemistry/modal_aero/seasalt_model.F90 modified: components/eam/src/chemistry/mozart/chemistry.F90 modified: components/eam/src/chemistry/mozart/lin_strat_chem.F90 modified: components/eam/src/chemistry/mozart/mo_gas_phase_chemdr.F90 modified: components/eam/src/chemistry/mozart/mo_photo.F90 modified: components/eam/src/physics/cam/hetfrz_classnuc_cam.F90 modified: components/eam/src/physics/cam/micro_mg_cam.F90 modified: components/eam/src/physics/cam/modal_aer_opt.F90 modified: components/eam/src/physics/cam/phys_control.F90 modified: components/eam/src/physics/cam/tropopause.F90

NEW code relevant to Explicit SOA not in other branches components/eam/src/chemistry/modal_aero/mam_soaexch_vbs.F90 components/eam/src/chemistry/modal_aero/modal_aero_data_amicphys.F90

tangq commented 1 year ago

Moving the discussion from email to here, so it can be helpful to others.

@quaz115 , I am not sure about the exact steps. Tagging @crterai and @wlin7 , who may know how to do it.

Hi Qi,

What are the next steps you suggest to proceed ?
Do we need to clone NGD_v3atm fork and re-merge SOA in that?

Quazi
crterai commented 1 year ago

@quaz115, @tangqi - It seems that the additional commits are coming from having merged the code into v2 before merging the code into the current branch and then rebasing. Is this correct? If that's so, I see two potential ways to get a clean PR. First, a) if Quazi has a branch that has not been rebased or merged with v2, you might be able to take that, ask Wuyin to copy it over to the v3atm fork, create a new branch, and rebase it on top of the NGD_v3atm branch. Or, b) a new branch created from the current NGD_v3atm branch - with the appropriate name indicating that it is related with SOA. Then, Quazi can cherry-pick, from the beginning, the commits associated with SOA from the mkshriva/atm/chemUCI_SOA_Merged_rebase1 branch into the new branch. I'm guessing it will still require some new commits to be made for commits that touch existing commits. Here's a page that describes cherry-picking in git, but others exist too: https://www.atlassian.com/git/tutorials/cherry-pick Both methods should keep all keep the git history from the development. @wlin7 - do you have any thoughts?

quaz115 commented 1 year ago

@crterai @tangq Yes, that's correct, the additional commits are coming because the code was merged into v2 before merging into the current branch

mingxuanwupnnl commented 1 year ago

@crterai I agree and would also suggest the latter one.

b) a new branch created from the current NGD_v3atm branch - with the appropriate name indicating that it is related with SOA. Then, Quazi can cherry-pick, from the beginning, the commits associated with SOA from the mkshriva/atm/chemUCI_SOA_Merged_rebase1 branch into the new branch.

@quaz115 I would like to confirm that your SOA treatment does not add new aerosols species in MAM4 right? Based on my read, you are still using current soa species (soa_a1-3, soa_c1-c3), right?

wlin7 commented 1 year ago

@quaz115 , both a) and b) @crterai proposed may work. I am not 100% certain of your explanation below. Your branch started from mingxuanwupnnl/atm/chemUCI-chemMZT-MOSAIC-MAM5_AMIP, then (1) added commits. (2) merged v2 into it instead of merging it into v2? (3) Additional commits after that? (4) Then rebasing with which branch? (5) Any more commits after rebasing?

If (3) and (5) didn't happen, the approach a) would be an easier way if you save the state before (2). Otherwise the approach b) sounds cleaner. If (1) didn't happen, still the approach b) is cleaner.

Yes, that's correct, the additional commits are coming because the code was merged into v2 before merging into the current branch

quaz115 commented 1 year ago

@crterai I agree and would also suggest the latter one.

b) a new branch created from the current NGD_v3atm branch - with the appropriate name indicating that it is related with SOA. Then, Quazi can cherry-pick, from the beginning, the commits associated with SOA from the mkshriva/atm/chemUCI_SOA_Merged_rebase1 branch into the new branch.

@quaz115 I would like to confirm that your SOA treatment does not add new aerosols species in MAM4 right? Based on my read, you are still using current soa species (soa_a1-3, soa_c1-c3), right?

@mingxuanwupnnl Yes, our SOA treatment does not add new aerosols species in MAM4, it still uses current soa species (soa_a1-3, soa_c1-c3) [We only added Explicit SOAG precursors: SOAG0, SOAG15 etc.]

quaz115 commented 1 year ago

@quaz115 , both a) and b) @crterai proposed may work. I am not 100% certain of your explanation below. Your branch started from mingxuanwupnnl/atm/chemUCI-chemMZT-MOSAIC-MAM5_AMIP, then (1) added commits. (2) merged v2 into it instead of merging it into v2? (3) Additional commits after that? (4) Then rebasing with which branch? (5) Any more commits after rebasing?

If (3) and (5) didn't happen, the approach a) would be an easier way if you save the state before (2). Otherwise the approach b) sounds cleaner. If (1) didn't happen, still the approach b) is cleaner.

Yes, that's correct, the additional commits are coming because the code was merged into v2 before merging into the current branch

@wlin7 My branch started from *CORRECTION (@wlin7 ) mingxuanwupnnl/atm/chemUCI-chemMZT-MOSAIC_AMIP (made SOA relevant updates to it of course, to create mkshriva/atm/chemUCI_SOA_Merged_rebase1, which i think got passed to you to put on v3atm fork ), then i followed the following steps:

Create a directory for the fork repo cd into directory and clone v3atm fork: git clone git@github.com:E3SM-Project/v3atm.git cd ./v3atm git fetch git checkout mkshriva/atm/chemUCI_SOA_Merged_rebase1 git submodule update --init --recursive

wlin7 commented 1 year ago

Thanks @quaz115 . The path is clear after moving to v3atm fork. The key is in the steps taken when working on the branches in the main E3SM repo. I feel you used multiple branches before settling things with branch mkshriva/atm/chemUCI_SOA_Merged_rebase1. Your commit history in the branch looks very clean, after Mingxuan's May 23 commit. I speculate you might have squashed a good number of commits to get there. So if taking path (b) as Chris proposed, then cherry-pick the few commits (essentially four, Jul 7, 8, Sep 2 and the 2nd one of Sep 13), it would end up the same just like the current branch, which is not useful to do. Since there is only a few commits, you can try anyway and see how it fares.

Another way is to create a new branch off the current one, mkshriva/atm/chemUCI_SOA_Merged_rebase1, rebase with NGD_v3atm, resolve the conflicts as git informs, then create a new PR to merge to NGD_v3atm. This would be essentially the same as merging the current PR to NGD_v3atm as is, but allowing you to resolve the conflicts first. The changes shown in the new PR can be inspected for a final time by our chem/SOA experts.

Just my 2 cents. All please chime in to see which path works better.

[ Added: when taking path (b), may encounter conflicts during cherry-picking. By resolving conflicts following each cherry-pick, it wouldn't end up like the current branch ]