Workflow-supplied options to be checked if already given in the config

[GarethCabournDavies]

Options given in the config and then from the workflow generator could clash. This is now being checked and an error is raised.

Some plumbing has gone in to the option-adding parts of this work to allow an option to be given multiple times, e.g. --statistic-files, for which some of the options come from the config, others from the workflow. I think this is the only example of that use case

Standard information about the request

This is a bug fix This change affects workflow generation, so all areas the live search.

This change follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines

Motivation

Supplying the option in both the config and from the workflow generator will have unknown/unforeseen outcomes.

Contents

• Add a check into the add_opt function if the keyword argument check_existing_options is True, this is the default.
• Pass this through from other places which call add_opt using **kwargs
• Ensure in the case that a list is being passed that only the --whatever is being checked
• Turn this check off for the --statistic-files option, which is the only place I can see at the moment which requires this

Links to any issues or associated PRs

1418

Testing performed

I ran the pycbc_make_offline_search_workflow code with gps-start-time given in the config overrides. This caused a failure. The workflow generator completed otherwise

• [x] The author of this pull request confirms they will adhere to the code of conduct

[GarethCabournDavies]

I will make the change to put it into pegasus_workflow.

I'll profile the workflow generation, but my thoughts on this are:

• The if x in list only works on the list of options for a specific node, so should not be too large.
• Thinking about which jobs will have large input lists, the HDF_TRIGGER_MERGE has many inputs in a large workflow, we could cut the list down to begin with by e.g. filtering the list according to list_value.startswith('--'), but that may just be equivalent to the if x in list
  • The statmap jobs could have quite a few options depending on how much findtrigs is parallelised, but I dont think that will cause problems in the if x in list
  • I can't think of other jobs with lots of inputs off the top of my head

[spxiwh]

Did you profile this as discussed for a "production scale" offline workflow?

[GarethCabournDavies]

Looking at the profile for an o4 chunk, both with this change: https://ldas-jobs.ligo.caltech.edu/~gareth.cabourndavies/testoutput/various_tests/workflow_config_clash/offline_wflow_o4_chunk1.png

and without it: https://ldas-jobs.ligo.caltech.edu/~gareth.cabourndavies/testoutput/various_tests/workflow_config_clash/offline_wflow_o4_chunk1_v242.png

We can see that the usage of add_input_opt and add_input_list_opt are practically unchanged

I think this is as add_input_list_opt doesn't run the comparison for the inputs. So though there is a [if x in list] check for each value, it isn't run for the input files, which would cause a problem for e.g. the hdf_trigger_merge