Move all machine-dependent path settings to a developer's configuration file about esmvaltool

[mattiarighi]

This is now implemented in REFACTORING_backend_developconfig.

@valeriupredoi and @bouweandela: it would be good if you could test it on your machines.

@bouweandela: please have a look at the error messages, I am not sure what is the best practice here.

@axel-lauer: you could try to add CMIP3 as a test, I will send you a config file for our cluster.

Thanks!

[valeriupredoi]

Hi Mattia, I have just committed a fix in data_finder.py (and added host_root in config.yml) for handling data finding and retrieval when using data servers like BADC or DKRZ in REFACTORING_backend, you may want to pull that out before testing the new machine config. I'll grab the branch and start testing, cheers for starting it! Cheers, V

…

On Fri, Sep 22, 2017 at 1:08 PM, Mattia Righi ***@***.***> wrote: This is now implemented in REFACTORING_backend_developconfig. @valeriupredoi <https://github.com/valeriupredoi> and @bouweandela <https://github.com/bouweandela>: it would be good if you could test it on your machines. @bouweandela <https://github.com/bouweandela>: please have a look at the error messages, I am not sure what is the best practice here. @axel-lauer <https://github.com/axel-lauer>: you could try to add CMIP3 as a test, I will send you a config file for our cluster. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#88 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AbpCo-fPxs1-mBmH_6xgwJSAfQca6qWtks5sk6MjgaJpZM4POQZ-> .

-- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

[mattiarighi]

Hi Valeriu, I think these fixes do not apply to REFACTORING_backend_developconfig, since the root paths definitions have been moved out of the code.

[valeriupredoi]

Mmm okay, the fixes deal with the drs-ing correctly, I'll check out the branch now and have looksy :) Will let you know as soon as I run a test! Cheers and have a good weekend if you don't hear back from me til Monday :) V

…

On Fri, Sep 22, 2017 at 3:22 PM, Mattia Righi ***@***.***> wrote: Hi Valeriu, I think these fixes do not apply to REFACTORING_backend_ developconfig, since the root paths definitions have been moved out of the code. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#88 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AbpCoyOlzdrHyhJLX1XR3UYvuvTFMDxdks5sk8KngaJpZM4POQZ-> .

-- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

[bouweandela]

Hi Mattia,

Looks good, you manage to do the same thing with 500 lines of code less!

Testing

it works fine with namelist_MyVar.yml, but I had to change config-user.yml a bit to make it work, did you forget to commit the updated config-user.yml file?

Error messages

I think this covers two topics: (1) logging and raising exceptions and (2) how to do string formatting

Logging and raising exceptions

I think we may need to define a few custom exceptions that we can raise if a user does something wrong. As far as I can see there are four places where we depend on user input:

1. config-user.yml
2. input data
3. namelist
4. diagnostic code

so I think we could add four custom exceptions and for these cases make sure we check the user input and we raise those if the user does something wrong and catch them with nice error message in the log. Probably all of those exceptions would need to inherit from a single ESMValToolError exception.

Apart from that, I think we do not need to raise many exceptions, just log what you are doing and run your code assuming everything is fine. E.g. I think that this style of code:

logger.info("CMIP5: mapping mip %s to realm", mip)
return mipdict[mip]

is preferred to this:

if mip in mipdict:
    return mipdict[mip]

raise KeyError("CMIP5: cannot map mip {} to realm".format(mip))

because it is shorter and therefore much more readable and it is often very difficult to cover all the possible things that can go wrong (and code becomes very difficult to read if you try to do this).

String formatting

• Best practice is to always use new style string formatting (i.e. using the format method of a string), see https://docs.python.org/3.6/library/string.html#format-examples for examples
• Except in log messages, there the format string should be specified using old style formatting, but the formatting arguments should be passed as arguments to the logging function. For example:
  yes: logger.info("x has value %s", x)
  no: logger.info("x has value %s" % x)
  no: logger.info("x has value {}".format(x))

[valeriupredoi]

Hey guys, Was just about to send the 'it works' confirmation email myself. Here's the cases I tested: in config-user_example.yml: rootpath: {CMIP5: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_Backend_PR21Sep2017/esmvaltool_pr21Sep17/testdatadrs, OBS: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_Backend_PR21Sep2017/esmvaltool_pr21Sep17/testdata} rootpath: {default: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_Backend_PR21Sep2017/esmvaltool_pr21Sep17/testdata} rootpath: {CMIP5: /badc/cmip5/data/cmip5/output1, OBS: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_Backend_PR21Sep2017/esmvaltool_pr21Sep17/testdatadrs} drs: {CMIP5: BADC} drs: None for different combinations of drs and rootpath. One thing I noticed is that one can not just say drs: {CMIP5: BADC} in the config file without specifying a valid BADC-like drs rootpath. We may want to introduce the case where the user wants to get the CMIP5 data off BADC without knowing where it lives exactly (knowing the rootpath) -- the previous data_finder did that. It's an easy fix, an option on top to check, if rootpath[CMIP5] is there or not. Apart from that, all seems a-okay :) Cheers, V

…

On Mon, Sep 25, 2017 at 2:37 PM, Bouwe Andela ***@***.***> wrote: Hi Mattia, Looks good, you manage to do the same thing with 500 lines of code less! Testing it works fine with namelist_MyVar.yml, but I had to change config-user.yml a bit to make it work, did you forget to commit the updated config-user.yml file? Error messages I think this covers two topics: (1) logging and raising exceptions and (2) how to do string formatting Logging and raising exceptions I think we may need to define a few custom exceptions that we can raise if a user does something wrong. As far as I can see there are four places where we depend on user input: 1. config-user.yml 2. input data 3. namelist 4. diagnostic code so I think we could add four custom exceptions and for these cases make sure we check the user input and we raise those if the user does something wrong and catch them with nice error message in the log. Probably all of those exceptions would need to inherit from a single ESMValToolError exception. Apart from that, I think we do not need to raise many exceptions, just log what you are doing and run your code assuming everything is fine. E.g. I think that this style of code: logger.info("CMIP5: mapping mip %s to realm", mip) return mipdict[mip] is preferred to this: if mip in mipdict: return mipdict[mip] raise KeyError("CMIP5: cannot map mip {} to realm".format(mip)) because it is shorter and therefore much more readable and it is often very difficult to cover all the possible things that can go wrong (and code becomes very difficult to read if you try to do this). String formatting - Best practice is to always use new style string formatting (i.e. using the format method of a string), see https://docs.python.org/3.6/ library/string.html#format-examples <https://docs.python.org/3.6/library/string.html#format-examples> for examples - Except in log messages, there the format string should be specified using old style formatting, but the formatting arguments should be passed as arguments to the logging function. For example: yes: logger.info("x has value %s", x) no: logger.info("x has value %s" % x) no: logger.info("x has value {}".format(x)) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#88 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AbpCo5jmQbZocbBUvjaT8w0RB-UbQPmKks5sl6yvgaJpZM4POQZ-> .

-- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

[valeriupredoi]

Hi Bouwe, The namelist should be thoroughly checked by interface_scripts/namelistchecks.py, that is in fact a developer's checklist only, since we don't really want the user to interfere with the namelist, unless it's a custom one, which we should start thinking about how we go about custom namelists. In any case, as I see it just now, config-user.yml is the only way we let the user mess about with and the rest should work via our stress testing. BTW we should add again run_diagnostic in the config, it works, but it's not there in the config anymore. Cheers, V On Mon, Sep 25, 2017 at 2:49 PM, Valeriu Predoi <[email protected]> wrote:

…

Hey guys, Was just about to send the 'it works' confirmation email myself. Here's the cases I tested: in config-user_example.yml: rootpath: {CMIP5: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_ Backend_PR21Sep2017/esmvaltool_pr21Sep17/testdatadrs, OBS: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_ Backend_PR21Sep2017/esmvaltool_pr21Sep17/testdata} rootpath: {default: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_ Backend_PR21Sep2017/esmvaltool_pr21Sep17/testdata} rootpath: {CMIP5: /badc/cmip5/data/cmip5/output1, OBS: /group_workspaces/jasmin/ncas_cms/valeriu/ESMValTool_Backend_PR21Sep2017/ esmvaltool_pr21Sep17/testdatadrs} drs: {CMIP5: BADC} drs: None for different combinations of drs and rootpath. One thing I noticed is that one can not just say drs: {CMIP5: BADC} in the config file without specifying a valid BADC-like drs rootpath. We may want to introduce the case where the user wants to get the CMIP5 data off BADC without knowing where it lives exactly (knowing the rootpath) -- the previous data_finder did that. It's an easy fix, an option on top to check, if rootpath[CMIP5] is there or not. Apart from that, all seems a-okay :) Cheers, V On Mon, Sep 25, 2017 at 2:37 PM, Bouwe Andela ***@***.***> wrote: > Hi Mattia, > > Looks good, you manage to do the same thing with 500 lines of code less! > Testing > > it works fine with namelist_MyVar.yml, but I had to change > config-user.yml a bit to make it work, did you forget to commit the updated > config-user.yml file? > Error messages > > I think this covers two topics: (1) logging and raising exceptions and > (2) how to do string formatting > Logging and raising exceptions > > I think we may need to define a few custom exceptions that we can raise > if a user does something wrong. As far as I can see there are four places > where we depend on user input: > > 1. config-user.yml > 2. input data > 3. namelist > 4. diagnostic code > > so I think we could add four custom exceptions and for these cases make > sure we check the user input and we raise those if the user does something > wrong and catch them with nice error message in the log. Probably all of > those exceptions would need to inherit from a single ESMValToolError > exception. > > Apart from that, I think we do not need to raise many exceptions, just > log what you are doing and run your code assuming everything is fine. E.g. > I think that this style of code: > > logger.info("CMIP5: mapping mip %s to realm", mip) > return mipdict[mip] > > is preferred to this: > > if mip in mipdict: > return mipdict[mip] > > raise KeyError("CMIP5: cannot map mip {} to realm".format(mip)) > > because it is shorter and therefore much more readable and it is often > very difficult to cover all the possible things that can go wrong (and code > becomes very difficult to read if you try to do this). > String formatting > > - Best practice is to always use new style string formatting (i.e. > using the format method of a string), see > https://docs.python.org/3.6/library/string.html#format-examples > <https://docs.python.org/3.6/library/string.html#format-examples> for > examples > - Except in log messages, there the format string should be specified > using old style formatting, but the formatting arguments should be passed > as arguments to the logging function. For example: > yes: logger.info("x has value %s", x) > no: logger.info("x has value %s" % x) > no: logger.info("x has value {}".format(x)) > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#88 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AbpCo5jmQbZocbBUvjaT8w0RB-UbQPmKks5sl6yvgaJpZM4POQZ-> > . > -- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

-- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

[mattiarighi]

Thanks for testing!
I will take note of your suggestions, but we should merge this now.
We have a working version which is a good basis for starting the integration of the diagnostics.

[valeriupredoi]

Yessir :) And Ive just coded up the directory structure with _NAMELIST_USERTAG_TIME (issue #15) so we don't overwrite and mess things up, I'll wait till you merge the branches into REFACTORING_backend, then Ill commit, that's just main.py that changed. Cheers, V

…

On Tue, Sep 26, 2017 at 12:51 PM, Mattia Righi ***@***.***> wrote: Thanks for testing! I will take note of your suggestions, but we should merge this now. We have a working version which is a good basis for starting the integration of the diagnostics. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#88 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AbpCo846sy-tceIhkS1NlztxIkhlCftSks5smOVFgaJpZM4POQZ-> .

-- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

[mattiarighi]

This is merged now.
Issue closed. :-)

[valeriupredoi]

Hey guys -- Issue #15 sorted out: now the code builds a set of different directories everytime user calls main eg plots_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 plots_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 preproc_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 preproc_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 tmp_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 tmp_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 work_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 work_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 where Testing is a user_tag specified by user in config file; if not specified it defaults to ESMValTool Now the user can specify any sorts of paths in config (e.g. plot_dir: ../../plots or plot_dir: ) they dont have to be absulye paths, the code converts them to absolute paths. The reason why I use uuid hexes (and feel free to chop on however many characters you think it's pretty) is that initially I used a utcnow datetime object ie YYMMDD_HHMMSS, but I spent the good part of today scratching my head because even if the directory tree was being built correctly, the NCL diagnostics were failing like mad - sometimes references were written, sometimes not, sometimes perfmetrics was complaining about a line on the diagnostics script, sometimes about another one. So I arrived to the conclusion that the datetime object is unrealiable and is messing up the environmental variables (I think I know which one -- the TZ timezone). The UUID hex identifier works like a charm, tested it thoroughly. Cheers, V

…

On Tue, Sep 26, 2017 at 1:12 PM, Mattia Righi ***@***.***> wrote: Closed #88 <#88>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#88 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AbpCo3LN6JeIM9JkDJQSXxDB--elvxM-ks5smOowgaJpZM4POQZ-> .

-- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

[valeriupredoi]

aah and another reason why not to use a fixed time identifier is that when we will make ESMValTool parallelizable, chances are the scheduler can start two jobs at exactly the same second :) Cheers, V On Tue, Sep 26, 2017 at 4:54 PM, Valeriu Predoi <[email protected]> wrote:

…

Hey guys -- Issue #15 sorted out: now the code builds a set of different directories everytime user calls main eg plots_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 plots_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 preproc_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 preproc_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 tmp_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 tmp_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 work_namelist_perfmetrics_Testing_a0bb4f48d7924ee0bec122453eb59103 work_namelist_perfmetrics_Testing_f75249145bae48dd8cbd9a97fbac2df4 where Testing is a user_tag specified by user in config file; if not specified it defaults to ESMValTool Now the user can specify any sorts of paths in config (e.g. plot_dir: ../../plots or plot_dir: ) they dont have to be absulye paths, the code converts them to absolute paths. The reason why I use uuid hexes (and feel free to chop on however many characters you think it's pretty) is that initially I used a utcnow datetime object ie YYMMDD_HHMMSS, but I spent the good part of today scratching my head because even if the directory tree was being built correctly, the NCL diagnostics were failing like mad - sometimes references were written, sometimes not, sometimes perfmetrics was complaining about a line on the diagnostics script, sometimes about another one. So I arrived to the conclusion that the datetime object is unrealiable and is messing up the environmental variables (I think I know which one -- the TZ timezone). The UUID hex identifier works like a charm, tested it thoroughly. Cheers, V On Tue, Sep 26, 2017 at 1:12 PM, Mattia Righi ***@***.***> wrote: > Closed #88 <#88>. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#88 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AbpCo3LN6JeIM9JkDJQSXxDB--elvxM-ks5smOowgaJpZM4POQZ-> > . > -- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"

-- Dr. Valeriu Predoi Computational Scientist for UKESM Core Team Department of Meteorology, University of Reading Earley Gate, Office 1U08 READING, RG6 6BB United Kingdom Mobile number: 07847416092 "If one day you be questioning your ability to come up with professional results, think of this: Noah's ark was built by farmers whereas the Titanic was crafted by skilled engineers"