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

Bugfix for correct awicm3 general version in choose block #1155

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

nwieters
Copy link
Contributor

@nwieters nwieters commented Apr 8, 2024

This PR corrects a small bugfix in awicm3.yaml for setting the right choose in a choose_general.version block.

@nwieters
Copy link
Contributor Author

nwieters commented Apr 8, 2024

A short description about what still fails in this branch and in branch awicm3_recom:

  • In awicm3.yaml the variable tidy_recipe: ${fesom.tidy_recipe} is set.
  • Because the fesom version is set with a variable subversion, the wrong fesom.yaml is taken (fesom.yaml instead of fesom-2.0.yaml)
    You can test this by masking the actual error by copying fesom-2.0.yaml -> fesom.yaml and look into the finished config: it says
fesom:
    ...
    debug_info:
        loaded_from_file: /work/ab0995/a270089/esm_tools/configs//components/fesom/fesom.yaml

Now it finds the variable tidy_recipe, but it takes the wrong fesom yaml config file.

  • In fesom.yaml there is no variable tidy.recipe set
  • This leads to an infinit loop searching for the variable tidy.recipe which gives the error
    RecursionError: maximum recursion depth exceeded while calling a Python object

I think the cause of this error is in the esm_parser function look_for_file line 234.

In the case of the fesom version set with a subversion variable (as in awicm3.yaml of branch awicm3_recom), at the time the actual fesom config file name is generated, the fesom version variable looks still like this fesom-2.0${subversion}. Without the variable ${subversion} evaluated. In line 234 of esm_parser.py it will cut off everything after and including the minus sign. So the fesom config file name will be fesom.yaml which leads to the above described error.

Copy link
Contributor

@mandresm mandresm left a comment

Choose a reason for hiding this comment

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

It should be working all now. @nwieters, can you give it another try?

This type of implementation was already done by Ying, @mbutzin and myself into AWIESM-2.1. One can find a lot of inspiration from the problems to come in AWICM3-REcoM in the configs/setups/awiesm/awiesm.yaml and in the configs/components/fesom/fesom-2.1.yaml specially if one searches for the with_recom.

@nwieters, merge whenever you consider is ready :)

@nwieters
Copy link
Contributor Author

nwieters commented Apr 9, 2024

Unfortunatly this doesn't solve the problem with the wrong fesom.yaml file taken.

@nwieters nwieters marked this pull request as ready for review April 10, 2024 09:17
@nwieters nwieters merged commit 93409af into awicm3_recom Apr 10, 2024
6 of 16 checks passed
@nwieters
Copy link
Contributor Author

nwieters commented Apr 10, 2024

Hi @chrisdane, @mandresm ,

I merged this PR into the branch awicm3-recom. It still does not solve the problem with the resolving the variable in the fesom version. But the workaround for this is to set the fesom version explicitly in the runscript

fesom:
    version: "2.0-recom"

With this I think @chrisdane can start to test awicm3-recom for now. I will fix the version issue then in release.

@mandresm
Copy link
Contributor

A very important remark for @chrisdane: once this is fix, one should not manually set the version of fesom (unless experimenting things) as specifying a particular version of AWICM3 should be enough, avoiding confusion and errors.

@mandresm mandresm deleted the awicm3_recom_nadine_bugfix branch April 10, 2024 09:56
@chrisdane
Copy link
Contributor

Thanks a lot all.

On 93409af the recom section now appears in the finished config of a check run. However, the namelist.recom is not in the run dir (the esm_runscripts call does not complain not to find namelist.recom). Any idea how to "activate" recom in this case?

Runscript:

/home/a/a270073/esm/runscripts/awicm3-v3.1.2_recom-levante.yaml

Expdir:

/work/ba1103/a270073/out/awicm3-v3.1.2-recom/test

Thanks a lot!
Chris

@mandresm
Copy link
Contributor

mandresm commented Apr 11, 2024

This is normal, the yaml file of awicm3 is not including the recom.yaml, that means no information from the default recom configuration is being used.

The solution is to follow a similar logic as in awiesm2 (use awiesm.yaml as a template) to include recom info into this coupled setup if with_recom is True:

'2.1-recom':
couplings:
- fesom-2.1-recom-par_tracers_mpi+echam-6.3.05p2-wiso+recom-2.0
add_include_models:
- recom

add_include_models will make sure that the info in recom.yaml is read.

recom:
choose_general.with_recom:
True:
version: "2.0"
#5.7.22 Ying: par-tracers only with ciso
use_ciso: true
add_choose_computer.name:
levante:
data_path: /home/a/a270105/initial_files/pi_init/dust_coremesh/
ollie:
data_path: /work/ollie/mbutzin/fesom2/input/dust/
restart_name: "restart."
remove_namelist_changes.namelist.recom.pavariables:
- constant_CO2
- firstyearoffesomcycle
- lastyearoffesomcycle
- numofCO2cycles
- currentCO2cycle
- REcoM_PI
remove_namelist_changes.namelist.recom.paco2_flux_param:
- CO2_for_spinup
namelist_dir: ${fesom.model_dir}/config/
"*":
not_used: ""

The box above covers more lines than it seems, so please scroll down on the box to see the whole thing. By adding a recom section you can modify in awicm3.yaml any recom information you need to modify without touching the default info in the recom.yaml. The choose is to make sure not to add recom information in simulations that don't run with recom. You probably don't need all of that, this is just so you have an idea how things could be done.

@mandresm
Copy link
Contributor

mandresm commented Apr 11, 2024

@chrisdane, let me know if we need to meet together with @nwieters to give you further advice on how to proceed, or if you think it would be good to discuss planning on support for adding this feature.

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.

Set the 'version' variable including other variables defined in choose_ blocks
3 participants