Initial Job Sizing Infrastructure

[TimothyWillard]

Describe your changes.

This pull request helps to move GH-487 along by adding:

• The BatchSystem enum to represent the three environments we expect to be running batch jobs on,
• The JobSize class to represent general properties of a batch job,
• The JobResources class to represent the general resource requirements of a batch job, and
• The JobTimeLimit class to represent the time limit a job is allowed to run for.

Spun out of GH-394.

Does this pull request make any user interface changes? If so please describe.

The user interface changes are new functionality added to gempyor.batch and are documented via docstrings but not in GitBook yet. I think it's not quite ready for GitBook documentation yet because:

1. These classes are much more developer oriented than user oriented, I expect that non-developers would never directly interact with these classes, and
2. I expect these classes to change some in response to adding resource estimation abilities (see task 3 of [Feature request]: flepimop submit Action For Submitting Calibration Jobs To HPC Environments #487).

What does your pull request address? Tag relevant issues.

This pull request addresses GH-487 by spinning out smaller infrastructure components from GH-394 to make that PR easier to review. With that said I expect these classes to change some between now and the final product for GH-487 when working on resource estimation for flepimop submit.

[saraloo]

Thanks Tim, looks good to me. From what I can tell this is just translating some of the old inference launcher script to this infrastructure? For the defaults here you've set:

nodes = 1 if nodes is None else nodes
cpus = 2 * job_size.jobs if cpus is None else cpus
memory = 2 * 1024 * job_size.simulations if memory is None else memory

Is this just the infrastructure you're building here, rather than specifics of these?

Also just to confirm some of the language here (can move this discussion elsewhere if better elsewhere, or to deal with later). For the R inference job = slot, and simulations = sims_per_job in the old inference launcher script? ie in the case of a single block simulations = iterations?
Idk if we care about it right now but for later when we harmonise across inference methods, we should stick to the same language. I think for emcee job=walker and simulations=niterations?

For emcee i also don't think we have any blocking capabilities afaik, and i don't think we use blocks on our HPCs - i think this was mainly an issue for AWS. A separate thought, but can consider (later) whether this is needed.

[pearsonca]

Not sure about the design here. Seems more like we should have a BatchSystem ABC rather than enum, and have BatchSystem's do their respective things?

[pearsonca]

why these flags? they are mutually exclusive, and the selection is captured by batch_system

[TimothyWillard]

Discussed offline, but adding note here for other reviewers.

flepiMoP/flepimop/gempyor_pkg/src/gempyor/batch.py

Lines 878 to 904 in ad11fae

	click.Option(
	param_decls=["--batch-system", "batch_system"],
	default=None,
	type=click.Choice(("aws", "local", "slurm"), case_sensitive=False),
	help="The name of the batch system being used.",
	),
	click.Option(
	param_decls=["--aws", "aws"],
	default=False,
	type=bool,
	is_flag=True,
	help="A flag indicating this is being run on AWS.",
	),
	click.Option(
	param_decls=["--local", "local"],
	default=False,
	type=bool,
	is_flag=True,
	help="A flag indicating this is being run local.",
	),
	click.Option(
	param_decls=["--slurm", "slurm"],
	default=False,
	type=bool,
	is_flag=True,
	help="A flag indicating this is being run on slurm.",
	),

The reason for this was to resolve these options from the prior pull request, GH-394, into a single consistent answer. Have removed for now and will rethink how to implement in a follow up PR to accommodate the dynamic batch systems added in 78d960a.

[TimothyWillard]

Is this just the infrastructure you're building here, rather than specifics of these?

These are just "defaults", I expect this to look different once working in the resource estimation abilities. I'm also open to changing defaults to whatever within reason.

Also just to confirm some of the language here (can move this discussion elsewhere if better elsewhere, or to deal with later). For the R inference job = slot, and simulations = sims_per_job in the old inference launcher script? ie in the case of a single block simulations = iterations?
Idk if we care about it right now but for later when we harmonise across inference methods, we should stick to the same language. I think for emcee job=walker and simulations=niterations?

Yeah, the JobSize class is designed to address GH-395 which is about creating consistent vocab for these concepts. The prior PR, GH-394, in fact used these as you suggest. See:

https://github.com/HopkinsIDD/flepiMoP/blob/ad11faea053045f6e61fab546ef9335b518ff726/flepimop/gempyor_pkg/src/gempyor/templates/inference_command.bash.j2

and

flepiMoP/flepimop/gempyor_pkg/src/gempyor/batch.py

Lines 1141 to 1154 in ad11fae

	# Job size
	job_size = JobSize.from_jobs_simulations_blocks(
	kwargs["jobs"],
	kwargs["simulations"],
	kwargs["blocks"],
	inference_method,
	batch_system,
	)
	logger.info("Preparing a job with size %s", job_size)
	if inference_method == "emcee" and job_size.blocks != 1:
	logger.warning(
	"When using EMCEE for inference the job size blocks is ignored, given %u.",
	job_size.blocks,
	)

[TimothyWillard]

I've removed references to specific inference methods from this PR for now, I think incorporating inference job type constraints are better suited for a follow-up PR.

[emprzy]

This looks great!