Conversation
…. Introduce env_pe and env_build as optional entries in simulation_setup. Introduce componentdict passing to clone_base_case to send correct parameter to correct component.
…e used in the initialisation files
…nd directly for more transparency in code
…file and routine for prestaging restart files if one is running hybrid or branch runs
…ging of restart files.
…ging of restart files.
Refactor config
…at you want as needed / wanted
Configure Codecov coverage reporting
…ecov-settings Revert "Configure Codecov coverage reporting"
Bumps [nbconvert](https://github.com/jupyter/nbconvert) from 7.16.6 to 7.17.0. - [Release notes](https://github.com/jupyter/nbconvert/releases) - [Changelog](https://github.com/jupyter/nbconvert/blob/main/CHANGELOG.md) - [Commits](jupyter/nbconvert@v7.16.6...v7.17.0) --- updated-dependencies: - dependency-name: nbconvert dependency-version: 7.17.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bump nbconvert from 7.16.6 to 7.17.0
| prestage-ensemble = "tinkertool.scripts.create_ppe.main:prestage_ensemble_CLI" | ||
| submit-ppe = "tinkertool.scripts.create_ppe.main:submit_ppe_CLI" | ||
| generate-paramfile = "tinkertool.scripts.generate_paramfile.main:main" | ||
| aerosol-ppe-cam-nl = "usermods.aerosol_ppe.scripts.aerosol_ppe_cam_nl_updates:main" |
There was a problem hiding this comment.
should useermods be part of a release?
There was a problem hiding this comment.
Agreed might not need be defined as an installable script.
|
|
||
| **namelist_control** - This section holds key-value pairs used for pointing to .ini files used to generate custom user_nl_<component> files. A valid section would be something like | ||
| ## namelist_control | ||
| - This section holds key-value pairs used for pointing to .ini files used to generate the user_nl_<component> which hold settings that are shared between all ensemble members. A valid section would be something like |
There was a problem hiding this comment.
I dont fully remember the behavior but if I remeber correctly it was program such that all things in the default namelist overwritten? If that's still the case it could be a usefull warning.
There was a problem hiding this comment.
The default namelist only overwritten if "f-string" parameters are used. Otherwise we only apped to the namelist file.
| optimization=None, | ||
| avoid_scramble=False, | ||
| params=None, | ||
| assumed_esm_component='cam', |
There was a problem hiding this comment.
I think this is a dangerous argument.
There was a problem hiding this comment.
Which argument in particular?
| err_msg = f"build_ppe returned 'None' but list of cases was expected since build_base_only={build_config.build_base_only}" | ||
| logging.error(err_msg) | ||
| raise RuntimeError(err_msg) | ||
| else: |
There was a problem hiding this comment.
else not needed if RuntimeError is raised
| raise RuntimeError(err_msg) | ||
| else: | ||
| # build_base_only=True, so no cases to submit | ||
| logging.info(">> Base case built successfully. No ensemble cases to submit.") |
There was a problem hiding this comment.
is there a system for the number of ">" ?
There was a problem hiding this comment.
No, Perhaps something to consider going forward.
|
|
||
| if not check_build_success: | ||
| logging.error(unsuccessful_build_msg) | ||
| return |
There was a problem hiding this comment.
raise some error instead of return?
There was a problem hiding this comment.
Check.build does not really work that well currently. E.g. if you do clone case, then check.build will fail. How it should work is that if create_clone is used, the check for build success only in the base_case.
| >> for details on the commands, run 'check-build --help', 'prestage-ensemble --help' and 'submit-ppe --help' | ||
| """ | ||
|
|
||
| def create_ppe(config: CreatePPEConfig): |
There was a problem hiding this comment.
Unsure of the behavior of logging if log_mode is write. Does every config object, e.g. check_build_config = CheckBuildConfig(
cases=cases_for_check_build,
verbose=config.verbose,
log_dir=config.log_dir,
log_mode=config.log_mode
)
make their own log file in the log_dir or do they go for the same file? if it's the same file an log_mode is write it would overwrite?
There was a problem hiding this comment.
perhaps add docstrings to all this functions as they are a direct entry point
| log_info_detailed('tinkertool_log', f"Building ensemble {i} of {checked_config.num_sims}") | ||
| ensemble_idx = f"{i:03d}" | ||
| tempParamDataset = checked_config.paramDataset.isel({checked_config.pdim: idx}) | ||
| # Special treatment for chem_mech.in changes: |
There was a problem hiding this comment.
perhaps the special treatment stuff deserves another level of abstraction, i.e. pulling it out into a new function. Makes it somewhat more modular and easy to read/follow
Johannesfjeldsaa
left a comment
Johannesfjeldsaa
left a comment
There was a problem hiding this comment.
Well done merging things! left some comments, some which is probably on my code as well. I am a little bit unsure if adding usermods to a version pr is good practice?
Also, note that I opened a PR to main-dev: #26. The changes are not that big but they are, in my opinion usefull and can be considered to be included in this version. Else I can change target to main after this one is pulled in.
|
@Johannesfjeldsaa Thanks for the review and helpfull suggestion. I'll go ahead and merge the changes into main for now. Then keep in mind the suggested improvement for the next release |
This is major update to Tinkertool.