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

Implement bromoform tracer and coupling #73

Merged

Conversation

tjiputra
Copy link
Collaborator

No description provided.

@JorgSchwinger
Copy link
Contributor

Hi @tjiputra and @DirkOlivie

I have pushed a commit adding to this pull-request.

  1. There was an issue with the UV absorption which was definitely wrong as it was (the absorption was only based on layer thickness, not on the actual depth). I have corrected this structurally, and I suggest to use the same principle as for the other radiation (calculating the average intensity over the layer). But I'm still not sure whether the parameters make sense, specifically referencing to 75 Wm-2 seems to be weird. I am not sure whether this was an ad-hoc approach that Hense and Quack used for their specific region. I think, we need to look more into this to confirm that the UV treatment is correct.

  2. Naming conventions: I change a bit of the naming of variables in the code. Since "vsls" is a group of species, variables referring to bromoform only should not be named using "vsls". I suggest to use "brf", since this also matches well with Mats style conventions used within BLOM. There are still a few instances that I couldn't change easily, namely the coupling indices

index_o2x_Faoo_fvsls_ocn
index_x2o_Sa_vslsprog

These I didn't change because they are used across the components, but I would also suggest to change their names to

index_o2x_Faoo_fbrf_ocn
index_x2o_Sa_brfprog

  1. I have also reduced the use of #ifdef BROMO a bit, since there were a few instance were it is not necessary (e.g. the fields for passing the brf-flux and atmbrf are always defined, so they can be always passed to hamocc).

I have tested that the code gives bit-identical results if the VSLS option is not activated. If you agree with my additions we can merge this pull request into the feature-hamocc-vsls branch.

@DirkOlivie
Copy link
Contributor

Hi @JorgSchwinger

Is your point (1) something specific for the VSLS implementation? Based on the fact that you write that tests without VSLS activated give bit identical results, I assume it is a VSLS-specific change.

I agree with your suggestion for the naming conventions.

@JorgSchwinger
Copy link
Contributor

Hi @DirkOlivie

yes, my point (1) refers to the treatment of the UV radiation in HAMOCC, i.e. the decay of bromoform due to UV in the water column.

This means that the sinks of bromoform in the water were not correctly represented so far.

@JorgSchwinger
Copy link
Contributor

@tjiputra

One more comment to the bromoform UV-sink in HAMOCC: variable "strahl" is the radiation intensity (Wm-2) over a wider spectral range, not only UV. For photosynthesis, we assume that a fraction 0.4 of this energy is PAR (implemented by multiplying pi_alpha by 0.4 in beleg). This means in turn,that maximum 0.6 of "strahl" can be UV light.

@tjiputra
Copy link
Collaborator Author

Hi @JorgSchwinger, @DirkOlivie

@JorgSchwinger, thank you for the thorough review. I made a note on the 75 Wm-2 on the paper, and assumed 'instantaneous surface irradiance' defined in Henze and Quack, 2000) to be the total irradiation. Now that I read it again, it makes sense to consider only the UV part. I am not sure if we can assume 0.6 of strahl is UV. I also have no quick solution to for the 75Wm-2. Should we compute climatology values from reanalysis fields and prepare as input file?

@DirkOlivie, I will update the naming in cime files to follow Jorg's suggestion. Jorg, could you do the same for blom:
old:
index_o2x_Faoo_fvsls_ocn
index_x2o_Sa_vslsprog

new:
index_o2x_Faoo_fbrf_ocn
index_x2o_Sa_brfprog

Thank you.

@DirkOlivie
Copy link
Contributor

Hi @JorgSchwinger , @tjiputra

Ok. I can have a look at the naming conventions in CAM, and update them.

@JorgSchwinger
Copy link
Contributor

Hi @tjiputra and @DirkOlivie

I have now changed the names of coupling indices in BLOM. I'll merge this pull request now.

@@ -330,10 +330,14 @@ subroutine restart_wt
call wrtrst('ficem_da',trim(c5p)//' k2 time',ficem_da,ip)
call wrtrst('abswnd_da',trim(c5p)//' k2 time',abswnd_da,ip)
call wrtrst('atmco2_da',trim(c5p)//' k2 time',atmco2_da,ip)
call wrtrst('atmbrf_da',trim(c5p)//' k2 time',atmbrf_da,ip) ! not read in restart_rd, necesarry?
Copy link
Contributor

Choose a reason for hiding this comment

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

@matsbn @tjiputra

Is it necessary that atmbrf_da is written to restart? If it is, then it needs to be also read from the file (which is not the case now). For a fully prognostic variable it is probably necessary? Also, if it is necessary, then it would be also necessary for DMS?

@DirkOlivie
Copy link
Contributor

Hi @tjiputra,

in CAM there are 3 files of which I don't know whether I should also change "vsls" into "brf" (listed below). I think it depends a bit on how cime will be changed (for 1 and 2), and what type of compsetnaming we will use (for 3)

  1. bld/build-namelist
    add_default($nl, 'flds_vslsa', 'val'=>'.false.');
    add_default($nl, 'flds_vslsc', 'val'=>'.false.');

  2. bld/namelist_files/namelist_definition.xml
    <entry id="flds_vslsa" type="logical" category="driver"
    <entry id="flds_vslsc" type="logical" category="driver"

  3. cime_config/config_component.xml
    dms_source='ocean_flux',vsls_source='ocean_flux'
    co2_cycle_rad_passive=.true.,dms_source='ocean_flux',vsls_source='ocean_flux'

@matsbn
Copy link
Contributor

matsbn commented Mar 22, 2021

Is the "#ifdef BROMO" needed in mod_forcing.F90 and restart_wt.F? It seems to be similar as flxco2 and flxdms which also is not always used. I prefer to keep ifdefs at a minimum. Also, will BLOM with these changes work with CIME versions that have not implemented bromoform tracer related exchange fields? If there is a dependency, it is important that we have a NorESM Externals.cfg somewhere that points to consistent branches/tags.

@JorgSchwinger
Copy link
Contributor

Hi @tjiputra,

in CAM there are 3 files of which I don't know whether I should also change "vsls" into "brf" (listed below). I think it depends a bit on how cime will be changed (for 1 and 2), and what type of compsetnaming we will use (for 3)

1. bld/build-namelist
   add_default($nl, 'flds_vslsa',     'val'=>'.false.');
   add_default($nl, 'flds_vslsc',     'val'=>'.false.');

2. bld/namelist_files/namelist_definition.xml
   <entry id="flds_vslsa" type="logical"  category="driver"
   <entry id="flds_vslsc" type="logical"  category="driver"

3. cime_config/config_component.xml
    dms_source='ocean_flux',vsls_source='ocean_flux' 
    co2_cycle_rad_passive=.true.,dms_source='ocean_flux',vsls_source='ocean_flux'

Hi @DirkOlivie

I think for all these cases it is fine to keep the "vsls". We might add more vsls-species later on, but these will then be switched on or off together with bromoform. So the general name in compset and namelists should contain "vsls". Just when it comes to names refering to individual species (like for the coupler indicies), I thought it is better to use individual species names.

@JorgSchwinger
Copy link
Contributor

Is the "#ifdef BROMO" needed in mod_forcing.F90 and restart_wt.F? It seems to be similar as flxco2 and flxdms which also is not always used. I prefer to keep ifdefs at a minimum. Also, will BLOM with these changes work with CIME versions that have not implemented bromoform tracer related exchange fields? If there is a dependency, it is important that we have a NorESM Externals.cfg somewhere that points to consistent branches/tags.

I agree it shouldn't be needed. It is always defined so it can be set to zero even if it is not used/needed. I will remove this.

Not sure whether this works without the changes in cime/cam. This needs to be considered before merging into master, but for now we only want to merge it into a feature branch in the main repository.

@JorgSchwinger JorgSchwinger merged commit e96c09a into NorESMhub:feature-hamocc-vsls Mar 22, 2021
@JorgSchwinger
Copy link
Contributor

Hi @tjiputra @DirkOlivie @matsbn

you have probably been notified that I have now merged the pull request into the branch feature-hamocc-vsls.

Since it is going into a feature branch this shouldn't be critical, and it is easier to continue working from there.

@matsbn
Copy link
Contributor

matsbn commented Mar 22, 2021

Sounds good @JorgSchwinger!

@DirkOlivie
Copy link
Contributor

I have now updated the feature-hamocc-vsls branch in the CAM repository with the suggested changes :

  • index_o2x_Faoo_fvsls_ocn has been replaced by index_o2x_Faoo_fbrf_ocn
  • index_x2o_Sa_vslsprog has been replaced by index_x2o_Sa_brfprog

@DirkOlivie
Copy link
Contributor

@JorgSchwinger @tjiputra @matsbn

I have now tried to launch a coupled simulation with Bromoform. It crashes after 1 month, probably when writing NetCDF output. It is not clear whether it happens in the atm or the ocn.

  • In the ocn.log is written : NetCDF : Variable not found
  • In the atm.log there is no error message
  • In the cesm.log : it looks that there is possibly a problem in ocn or atm.

I have put the logfiles on betzy in /cluster/projects/nn9560k/olivie/shared_files
The case is in /cluster/projects/nn9560k/olivie/cases-keyclim-test/N1850frc2_bromo_f19_tn14_20210322_test1
The model code is in : /cluster/projects/nn9560k/olivie/shared_files/2021-03-22-NorESM-NorESMhub-vslstest

Any idea what might be the reason for the crash?

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.

4 participants