Missing optional parameters in experiment.yaml for 3dfgat_cycle

[mranst]

Description

This (maybe just partially) addresses issue #440 . In the case of 3dfgat_cycle, which seems to be the example mentioned in the issue, the consideration of cice6_domain in the experiment file is conditional on "cice6" being present in marine_models, as specified in that suite's flow.cylc.

swell/src/swell/suites/3dfgat_cycle/flow.cylc

Lines 221 to 232 in 2b0f03b

	{% if 'cice6' in marine_models %}
	[[RunJediConvertStateSoca2ciceExecutable-{{model_component}}]]
	script = "swell task RunJediConvertStateSoca2ciceExecutable $config -d $datetime -m {{model_component}}"
	platform = {{platform}}
	execution time limit = {{scheduling["RunJediConvertStateSoca2ciceExecutable"]["execution_time_limit"]}}
	[[[directives]]]
	{%- for key, value in scheduling["RunJediConvertStateSoca2ciceExecutable"]["directives"][model_component].items() %}
	--{{key}} = {{value}}
	{%- endfor %}
	{% endif %}

In setting up the experiment, two sweeps of flow.cylc are, done using the function template_string_jinja2, which compares flow.cylc and a dictionary set up by suite and task questions. marine_models is a model-dependent variable, so it's actually addressed by the second, more exhaustive sweep.

swell/src/swell/deployment/prepare_config_and_suite/prepare_config_and_suite.py

Lines 431 to 439 in 2b0f03b

	# 7. Perform a more exhaustive resolving of suite file templates
	# --------------------------------------------------------------
	# Note that we reset the suite file to avoid templates having been left unresolved
	# (removed) from the previous attempt. We still do not ask for an exhaustive resolving
	# of templates because there are things related to scheduling that are not yet able to be
	# resolved. In the future it might be good to bring some of that information into the
	# sphere of suite questions but that requires some careful thought so as not to overload
	# the user with questions.
	suite_str = template_string_jinja2(self.logger, self.suite_str, self.experiment_dict,

The issue here is that the marine_models specified in the question dictionary are actually part of a sub-dictionary, so it's not resolved in the jinja2 sweep. This change just points marine_models to the right place, so. cice6_domains is triggered to add to experiment.yaml. I think this issue is linked to a broader discussion of the configuration files, so I'm not sure if it completely addresses the issue, but the behavior in this case seems to be fixed

[Dooruk]

Thanks @mranst! Sorry this was caused by a tiny bug on my part. I agree with your comment on the more advanced configuration changes in the future but this resolves #440, since there is already #314.

On another note, is there a practical reason to work on your fork?

[mranst]

Thanks @mranst! Sorry this was caused by a tiny bug on my part. I agree with your comment on the more advanced configuration changes in the future but this resolves #440, since there is already #314.

On another note, is there a practical reason to work on your fork?

Why have a fork of swell that I contribute to rather than create a branch on the main repository? There's not really a reason other than that's how I've done things in the past. I've never really thought much about it, admittedly I don't yet have the strongest confidence on how things are done in GitHub

[ashiklom]

On another note, is there a practical reason to work on your fork?

Forking to your own GitHub account and then submitting pull requests from that is generally the "default" open source contribution workflow, because it doesn't require adding people as collaborators to a GitHub repository, and liberates folks to experiment more with branch naming, repo structure, etc. (because you don't have to worry about repository-specific things like branch protection, CODEOWNERS, etc.).

I think the main reason we prefer to work directly off the GEOS-ESM repo is that we don't have the ability to trigger Tier 1 tests via GH Actions for forks. This would make sense if our Tier 1 tests ran automatically when PRs were submitted. But, since our system only allows a small number of specific people to trigger those workflows, and only by hand, I think that policy is probably security overkill (because you should do a code review before triggering that action anyway, especially for unfamiliar contributors). But that's a question for @jardizzo.

[Dooruk]

Tier 1 actions is the main reason, but my question is mostly practical. If we were to collaborate on a branch I can't push changes directly to remote personal forks. I don't think I can even suggest as a PR (?).

Huh, I was able to merge develop to this fork on Github web. I didn't know I could do that, I thought it was going to reject.

[jardizzo]

Regarding forks and workflows: I'm not an expert on what can be done with the forks except to say that it makes sense that workflows are not going to launch once the repo is forked. I'm a little concerned that I have not adequately conveyed the workflow mechanism however. There is no limit on the range of triggers for the workflows. We can change the workflows to automatically run no matter who makes the PR request. Manual triggers are an option but not at the exclusion of automatic triggers. Dan had the idea that we could make the workflows require a label assignment as part of the credentials for triggering a workflow. We can remove that requirement and give permission to all or a large group of users to execute the on-prem workflows. I don't think we want too many Tier2+ workflows executing however. Just the cheaper Tier-1 workflows. I'm happy to discuss more. I will review the trigger configuration now.

[jardizzo]

Here are the current triggers allowed:

on:
push:
branches:
- develop
workflow_dispatch:

The "workflow_dispatch" is the manual trigger. We can add pull_request and we can add constraints on what branches are allowed for each trigger (or branches to exclude). I can then add users to the NAMS check that we implemented. That will enable more users to execute the workflow.