SCALE-MS / scale-ms

SCALE-MS design and development
GNU Lesser General Public License v2.1
4 stars 4 forks source link

`scalems.executable()` Generic command line tool wrapper. #38

Open eirrgang opened 4 years ago

eirrgang commented 4 years ago

Refine the user interface and address functional requirements for tasks implemented in terms of external executables or shell command lines.

Evaluate the gmxapi.commandline_operation and RADICAL Pilot "executable" task expression to refine the interface by which users declare functions/commands/coroutines implemented in terms of CLI tools.

Functional areas and design considerations

gmxapi.commandline_operation

gmxapi 0.1 provides for positional arguments, named input arguments, and named output arguments. Named input and output arguments are ARGV sub-sequences that begin with a "flag" element (unique in the command line, since it is used as a dictionary key).

Implementation oversights include missing stream handling (resolved in a patch), poor error handling, poor user support for locating executables, and inadequate handling of the working directory and files.

Larger design deficiencies likely include inadequately flexible call signatures, but are most obvious in relation to parallel resource management and ensemble work flows. The model does not account for how executables may or may not be MPI-aware, and does not provide any mechanism for negotiating either (a) the subdivision of hardware resources visible from the process environment, or (b) collective execution of multiprocess tools.

RADICAL Pilot

@eirrgang is not prepared to summarize the use cases, strengths, or deficiencies of prior art in the RADICAL stack, but notes that there has been more focus on distributing wrappers specialized for particular tools in extension code layers. The development tools for such extensions may already be sufficiently general and nearly feature-complete, but it will be interesting to consider the lessons learned as we take the opportunity to freshly define a user-oriented interface.

References

Use cases

In trivial use cases, a command is a single thread in a single process, produces no artifacts, and returns valid results every time it is run. For more interesting use cases, we need to consider the (targeted subset of) general use cases from https://github.com/SCALE-MS/scale-ms/wiki/Execution-model#run-time-scenarios as relating to various command line tools and as reflected in their invocations.

Tracking

eirrgang commented 4 years ago

Discussion point

In RP,

In gmxapi

For SCALE-MS

andre-merzky commented 4 years ago

Note that the LFS parameter is optional and not enforced - we use it as scheduling hint to avoid oversubscription of local storage.

STDIO can be discarded also in RP - but we indeed find capture generally useful for debugging purposes.

eirrgang commented 4 years ago

Note that the LFS parameter is optional and not enforced - we use it as scheduling hint to avoid oversubscription of local storage.

The default value noted at https://radicalpilot.readthedocs.io/en/stable/apidoc.html#computeunitdescription is 0. Is 0 interpreted as infinity?

STDIO can be discarded also in RP - but we indeed find capture generally useful for debugging purposes.

https://radicalpilot.readthedocs.io/en/stable/apidoc.html#computeunitdescription states that the default behavior is to capture STDOUT and STDERR in files of the same names. Are you saying that this is separate from whether the results are staged back to the client, or are you saying that there is a way to disable capture entirely?

I think the behavior we want right now is to capture the output files, but to minimize blocking of dependent tasks if staging is not explicitly required.

andre-merzky commented 4 years ago

Note that the LFS parameter is optional and not enforced - we use it as scheduling hint to avoid oversubscription of local storage.

The default value noted at https://radicalpilot.readthedocs.io/en/stable/apidoc.html#computeunitdescription is 0. Is 0 interpreted as infinity?

The default is to ensure zero bytes of local storage - it is up to the application and the system to negtiate storage space.

STDIO can be discarded also in RP - but we indeed find capture generally useful for debugging purposes.

https://radicalpilot.readthedocs.io/en/stable/apidoc.html#computeunitdescription states that the default behavior is to capture STDOUT and STDERR in files of the same names. Are you saying that this is separate from whether the results are staged back to the client, or are you saying that there is a way to disable capture entirely?

I think the behavior we want right now is to capture the output files, but to minimize blocking of dependent tasks if staging is not explicitly required.

Well, one can specify /dev/null as target :-)

eirrgang commented 4 years ago

Proposed replacement interface for https://scale-ms.readthedocs.io/en/latest/python.html#scalems.cli is scalems.executable() (converging on asyncio.create_subprocess_exec compatibility) from https://github.com/SCALE-MS/scale-ms/commit/991c21cfbc29bd118872e816d61e357ced1bc7c4 (feel free to comment at the commit).

Updates to https://scale-ms.readthedocs.io/en/latest/python.html#scalems.commandline_operation will come later, and will benefit from more discussion on where/how to express run-time parameters that are not relevant to high-level task/data identity or uniqueness of results, or which are not portable between execution environments.

eirrgang commented 3 years ago

Some notes from an email conversation:

We need to be able to express input and output files that do not use CLI flags or which do not necessarily appear on the command line at all.

In the former case, it is trivial to embed references in the command line arguments directly. Otherwise, as a work-around, a thin wrapper can accept inputs that don't get passed explicitly to a wrapped call to scalems.executable.

Outputs are trickier.

Suggestion: Introduce a new concept (well, Peter and I have talked about it before) of a placeholder or "sink" object that can be created before its data source, but the output would have to be given a label and then retrieved from the Context rather than appearing as an attribute of the object returned by scalems.executable. I.e. the output file would be a separate workflow object whose addition to the workflow is delayed (under the hood) until the scalems.executable command has created its Subprocess task but before it returns the task reference to the caller.

Technically, this could just be a sort of adapter to the lower-level workflow editing protocol that would actually be pretty easy to implement in terms of the "awaitable" ideas.

But I think the first draft is just to implement the scalems.OutputFile with an additional label initialization argument that can be passed to the workflow manager (and then used as a key).

mrshirts commented 3 years ago

In the former case, it is trivial to embed references in the command line arguments directly.

When you say this, you mean the user will go through and indicate the names of the files that the input script counts on? This seems like it's potentially problematic since there might be typos/mismatches between what the script says and what the user remembers.

Note that one could write a helper script that scans the lammps input file and checks to see what the input files are asked for (there are only a limited number of commands that do this). I assume that's what you mean by: "Otherwise, as a work-around, a thin wrapper can accept inputs that don't get passed explicitly to a wrapped call to scalems.executable."

Outputs are trickier.

I'm not sure I quite followed the discussion above. Would a workaround for now be a similar short function to the one mentioned aboce that scans the lammps input commands for output files and then adds them explicitly to scalems.executable? Or would that even make sense, since the lammps command would ignore them. I guess the point is not to pass them to the lammps files, but just to register them with scale-MS so it knows about their existence.

Question: what do we do about random things like logfiles, or checkpoint files - is SCALE-MS aware of them? Does it need to be?

mrshirts commented 3 years ago

There are two reasons I can think of for scale-ms to know about input and output files. One is to be able to pass them to the binary. But if they are specified inside another input file, then they don't need to be passed to the binary. But the other reason, I assume, would be to register them with scale-ms, so it's aware of the existence of those files. Which is the purpose of including them in the executable call? Or both?

eirrgang commented 3 years ago

But if they are specified inside another input file, then they don't need to be passed to the binary.

Correct. Then they should be a dependency of the file they are specified in. That is the subject of another issue, but the dependency can also be expressed as an input of a wrapper function. I.e. A lammps simulation command may be written in terms of scalems.executable. The wrapper function can express the input file, but the author knows that the lammps command will look for a file of a specific name. We should express that in the call to scalems.executable, but we don't technically have to until we get the RP integration further along.

But the other reason, I assume, would be to register them with scale-ms, so it's aware of the existence of those files. Which is the purpose of including them in the executable call? Or both?

For the purposes of this issue, we need scalems to know about all of the input and output files involved with the command line tool. Some of this can be done through transitive dependencies in the future, but in the most basic case, we need to be able to express filesystem dependencies and artifacts at the same time we express everything else about a wrapped command line.

eirrgang commented 3 years ago

Sorry, I didn't notice that you had posted twice...

In the former case, it is trivial to embed references in the command line arguments directly.

When you say this, you mean the user will go through and indicate the names of the files that the input script counts on?

I was referring specifically to the case where a file is indicated on the command line, but without a flag. A file name could be passed in argv rather than in inputs.

This seems like it's potentially problematic since there might be typos/mismatches between what the script says and what the user remembers.

Yes, we should encourage the use of variables / Python objects instead of raw file names. We will be making incremental improvements to the ways we handle abstract references to files in the data flow for a while. In the short term, we should try to make sure that all references to a file are derived from the same Python variable and we can work from there.

Note that one could write a helper script that scans the lammps input file and checks to see what the input files are asked for (there are only a limited number of commands that do this). I assume that's what you mean by: "Otherwise, as a work-around, a thin wrapper can accept inputs that don't get passed explicitly to a wrapped call to scalems.executable."

My preference would be to substitute filenames programmatically rather than to derive filenames by scanning a text file, but what you propose is a reasonable first step if you find it easier. As a side note, I would consider that a special case of a make_input sort of step that converts a text file to a Python-level awareness of what the dependencies are, but we can separate out the logical components in later stages of refinement.

Outputs are trickier.

I'm not sure I quite followed the discussion above. Would a workaround for now be a similar short function to the one mentioned aboce that scans the lammps input commands for output files and then adds them explicitly to scalems.executable? Or would that even make sense, since the lammps command would ignore them. I guess the point is not to pass them to the lammps files, but just to register them with scale-MS so it knows about their existence.

To recap:

Question: what do we do about random things like logfiles, or checkpoint files - is SCALE-MS aware of them? Does it need to be?

We might need to decide that SCALE-MS should consider the entire working directory to be an artifact that should be archived in case it is needed, but the assumption we have been working under is that SCALE-MS only cares about files that it is told to care about, and only automatically transfers or blocks on a subset of those as needed to satisfy dependencies.

mrshirts commented 3 years ago

Would a workaround for now be a similar short function to the one mentioned aboce that scans the lammps input commands for output files and then adds them explicitly to scalems.executable?

Yes, that definitely can be done.

mrshirts commented 3 years ago

My preference would be to substitute filenames programmatically rather than to derive filenames by scanning a text file, but what you propose is a reasonable first step if you find it easier.

I'd have to think about this. Basically it would mean you can't write lammps scripts, you would have to write meta-lammps scripts. Which might be OK, but we would have to think carefully about the meta-language.

mrshirts commented 3 years ago

This is a noted shortcoming that needs to be addressed under the current issue.

Could this simply be done with just having scale_ms.executable take inputs_commandline and inputs_implict instead of inputs?, where inputs_implicit is just a list of files, rather than dictionary of commandline inputs / filenames?

Also, if it is a dictionary of command line inputs, then what about the case where the program can accept multiple instances of a filename (i.e. -f file1 -f file2 instead of -f file1 file2). However, I'm not sure any program we are working with does this, so it might be irrelevant.

mrshirts commented 3 years ago

We might need to decide that SCALE-MS should consider the entire working directory to be an artifact that should be archived in case it is needed, but the assumption we have been working under is that SCALE-MS only cares about files that it is told to care about, and only automatically transfers or blocks on a subset of those as needed to satisfy dependencies.

This makes sense. Keep track of everything in case needed for inspection later, but the logic of the workflow execution only depends on the things that SCALE-MS is told about.

eirrgang commented 3 years ago

I'd have to think about this. Basically it would mean you can't write lammps scripts, you would have to write meta-lammps scripts.

Ideally, users write scalems scripts. :-)

The maintainer of scalems.wrappers.lammps writes meta-lammps scripts.

Which might be OK, but we would have to think carefully about the meta-language.

That would be the design of the scalems.wrappers.lammps module. Alternatively, we can just use the strategy of formatted text futures, i.e. scalems.Text objects, allowing such objects to have input file dependencies and to be wrapped in objects that track output files. But I probably can't follow up on that until next week.

Could this simply be done with just having scale_ms.executable take...

Essentially, yes. I'm not sure the current dictionary passing is helpful, anyway. The alternative I propose (again, I think I wrote this down clearly somewhere, but I can't find it now...) is to do away with the dictionary inputs and outputs in their current form and to meet that use case by just annotating arguments directly. E.g.

from scalems import *
executable(['command', File('input'), '-o', OutputFile(label='out', suffix='.traj'])

At the same time, we can reintroduce key word arguments for files that do not appear in the command line.

from scalems import *
cmd = executable(
    ['someothercommand'],
    inputs=[File('implicitinput')])

Since we acknowledge that we probably need to provide a way to access an archive of the working directory, we might as well handle implicit output files with something like cmd.artifacts['implicitoutput'] and let the resulting data dependency get translated into output staging instructions for the executable task under the hood.

eirrgang commented 3 years ago

To summarize, the proposed signature for scalems.executable is

def executable(
    args: Sequence[Union[str, scalems.Reference]],
    *,
    inputs: Mapping[str, scalems.File] = None,
    outputs: Sequence[str] = ()
    stdin: scalems.File = None,
    environment: Mapping[str, str] = None,
    resources: Mapping[str, Any] = None,
    workflow: WorkflowManager = None
    ):
    ...

An example workflow item would look like

{
    'args': ['program', 'flag1', 'arg1', 'flag2', obj],
    'inputs': {'data': file_reference},
    'outputs': ('output1', 'output2'),
    'stdin': another_file_reference,
    environment: {},
    resources: {
        'cpus': 4,
        'mpi_ranks': 0
    },
}

and the associated Result would look like

{
    'stdout': file_reference1,
    'stderr': file_reference2,
    'outputs': {
        'output1': file_reference3,
        'output2': file_reference4
    },
    'exitcode': 0,
    'artifacts': archive_reference
}

artifacts probably doesn't need to be part of the initial implementation, but the idea would be to provide a convenient way to access an archive of the working directory after the task reached a final state. The data type could be an abstraction of a filesystem directory or arbitrary structured archive format, like zip, tar, tar.gz, tar.bz, etc.

peterkasson commented 3 years ago

That looks reasonable to me.

mrshirts commented 3 years ago

So the "other files" would be written as , i.e. inputs': {'data': file_reference}. So data would be the file name, and file_reference would be the scale_ms file reference? Does creating this input dictionary create the file reference, or do you have to create the file reference first?

eirrgang commented 3 years ago

inputs': {'data': file_reference}. So data would be the file name, and file_reference would be the scale_ms file reference?

Correct.

Does creating this input dictionary create the file reference, or do you have to create the file reference first?

There should not be any problem with creating the file reference inline. I expect the syntax would look like the following.

For a local file that the command will assume is in ./hardcodedfilename:

my_command = scalems.executable(
    ...,
    inputs={
        'hardcodedfilename': scalems.file(localfilepath)
    }
)

For a generated file from another command (my_dependency):

my_command = scalems.executable(
    ...,
    inputs={
        'hardcodedfilename': my_dependency.outputs['outputname']
    }
)

The same file reference syntax should be usable in elements of args. The syntax and details of file references will be addressed under #129, which can be considered blocking for the current issue.