Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to class-based configuration #133

Closed
annawoodard opened this Issue Mar 16, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@annawoodard
Copy link
Collaborator

annawoodard commented Mar 16, 2018

This was discussed at the meeting yesterday.

@annawoodard

This comment has been minimized.

Copy link
Collaborator Author

annawoodard commented Apr 17, 2018

For example, instead of

singleNode = {
  "sites": [
    {"site": "Local_IPP",
      "auth": {
        "channel": "ssh",
        "hostname": "swift.rcc.uchicago.edu",
        "username": os.environ['MIDWAY_USERNAME'],
        "scriptDir": "/scratch/{0}/parsl".format(USERNAME)
      },
      "execution": {
        "executor": "ipp",
        "provider": "slurm",
        "block": {  # Definition of a block
           "nodes": 1,            # of nodes in that block
           "taskBlocks": 1,       # total tasks in a block
           "initBlocks": 1,
           "maxBlocks": 1,
            "options": {
               "partition": "westmere",
               "overrides": '''module load python/3.5.2+gcc-4.8'''
            }
        }
      }
    }
 ],
 "globals": {"lazyErrors": True}
}

we could have something like

from parsl.config import Config
from libsubmit.providers.slurm.slurm import Slurm

user = os.environ['MIDWAY_USERNAME']

config = Config(
    [
        Site(
            'local_IPP',
            channel=SSHChannel(
                hostname='swift.rcc.uchicago.edu',
                username=user,
                script_dir='/scratch/{0}/parsl'.format(user)
            ),
            provider=Slurm(
                executor='ipp',
                nodes=1,
                task_blocks=1,
                init_blocks=1,
                max_blocks=1,
                partition='westmere',
                overrides=''''module load python/3.5.2+gcc-4.8'''
            )
        )
    ],
    globals={'lazy_errors': True}
)

One advantage would be that if someone handed off the above config to you, finding the relevant documentation interactively would be more natural:

>>> help(Slurm)

Help on class Slurm in module libsubmit.providers.slurm.slurm:

class Slurm(libsubmit.providers.provider_base.ExecutionProvider)

| Slurm Execution Provider
|
| This provider uses sbatch to submit, squeue for
| status and scancel to cancel jobs. The sbatch
| script to be used is created from a template file
| in this  same module.

Additionally, the documentation could be written more clearly, and defaults could be specified in the constructor signature (and so would be self-documenting) (left is current, right is proposed option)
screen shot 2018-04-17 at 6 27 48 pm

Another advantage would be that the proposed Config class could be completely specified in its own documentation in a straightforward way (which pieces are required, what defaults will be used, etc). Currently we lean heavily on showing examples rather than having a clear specification; I think we should have both.

Relevant user feedback from the help channel:

I don't know that I have anything specific, I just feel like I don't know how to do anything related to configuration haha
Like, I don't know what's required, what's possible, etc

This is related to Parsl/libsubmit#22; switching to class-based configs would allow the (currently non-existent) validation to be done more naturally in the constructors of each component, allowing for interactive validation in notebooks (i.e., you are not sure if your config is correct, so you instantiate an instance of Config and immediately get an error raised; you can keep tweaking things interactively until you have a valid config).

Here are a few slides from the meeting.

@annawoodard

This comment has been minimized.

Copy link
Collaborator Author

annawoodard commented Apr 17, 2018

Another advantage would be that we could get rid of redundant fields, for example here from the Slurm provider documentation, where the executor must be IPP:

     "executor"   : #{Description: Define the executor used as task executor,
                    # Type : String,
                    # Expected : "ipp",
                    # Required : True},
@benclifford

This comment has been minimized.

Copy link
Contributor

benclifford commented Apr 18, 2018

This is an opportunity to refactor configuration hierarchy, but that opportunity is distinct from it being python/json based.

I support this proposal (and for the reasons @annawoodard has described in slides) as long as the intention is that configurations are written in python land: for example, if it is expected that configurations are written by the same people writing python application code; or are being generated by python generation code.

(counterexamples: sysadmins writing configurations for their users - where neither is a python programmer; someone writing a config generator in a different language - I think those are quite weak examples, though)

@annawoodard

This comment has been minimized.

Copy link
Collaborator Author

annawoodard commented May 2, 2018

To flesh this out more, I propose that we remove dataflow/config_defaults.py and make sure that defaults are all specified in the appropriate constructor signature. Note we should also be able to remove the ExecProviderFactory since the executors can be instantiated in the config.

@annawoodard

This comment has been minimized.

Copy link
Collaborator Author

annawoodard commented May 15, 2018

After sketching this out, here is an example of a config comprising three executors.

current

config = {
    "sites": [
        {
            "site": "ipp_short",
            "auth": {
                "channel": "ssh",
                "hostname": "swift.rcc.uchicago.edu",
                "username": "annawoodard",
                "scriptDir": "/scratch/midway2/annawoodard/parsl_scripts"
            },
            "execution": {
                "executor": "ipp",
                "provider": "slurm",
                "block": {
                    "nodes": 1,
                    "taskBlocks": 4,
                    "initBlocks": 4,
                    "maxBlocks": 4,
                    "walltime": "00:05:00",
                    "options": {
                        "partition":
                        "westmere",
                        "overrides":
                        """module load python/3.5.2+gcc-4.8; source /scratch/midway2/yadunand/parsl_env_3.5.2_gcc/bin/activate"""
                    }
                }
            },
            "data": {
                "globus": {
                    "endpoint_name": "7d2dc622-2edb-11e8-b8be-0ac6873fc732",
                    "endpoint_path": "/home/janedoe/short"
                },
                "working_dir": "/home/janedoe/short",
            }
        }, {
            "site": "ipp_long",
            "auth": {
                "channel": "ssh",
                "hostname": "swift.rcc.uchicago.edu",
                "username": "annawoodard",
                "scriptDir": "/scratch/midway2/annawoodard/parsl_scripts"
            },
            "execution": {
                "executor": "ipp",
                "provider": "slurm",
                "block": {
                    "nodes": 1,
                    "taskBlocks": 1,
                    "initBlocks": 4,
                    "maxBlocks": 4,
                    "walltime": "00:60:00",
                    "options": {
                        "partition":
                        "westmere",
                        "overrides":
                        """module load python/3.5.2+gcc-4.8; source /scratch/midway2/yadunand/parsl_env_3.5.2_gcc/bin/activate"""
                    }
                }
            },
            "data": {
                "globus": {
                    "endpoint_name": "ddb59aef-6d04-11e5-ba46-22000b92c6ec",
                    "endpoint_path": "/home/janedoe/long"
                },
                "working_dir": "/home/janedoe/long",
            }
        }, {
            "site": "local_threads",
            "auth": {
                "channel": None
            },
            "execution": {
                "executor": "threads",
                "provider": None,
                "maxThreads": 8
            }
        }
    ],
    "globals": {
        "lazyErrors": True
    },
    "controller": {
        "publicIp": os.environ.get('PUBLIC_IP', None)
    }
}

proposed

from parsl.config import Config
from parsl.executors.ipp import IPyParallelExecutor
from parsl.executors.threads import ThreadPoolExecutor
from parsl.data_provider.endpoints import Globus

from libsubmit.providers.slurm.slurm import Slurm
from libsubmit.channels.ssh import SSHChannel

slurm = Slurm(
    channel=SSHChannel(
        hostname='swift.rcc.uchicago.edu',
        username='annawoodard',
        script_dir='/scratch/midway2/annawoodard/scripts'
    ),
    partition='westmere',
    overrides="module load python/3.5.2+gcc-4.8; source /scratch/midway2/yadunand/parsl_env_3.5.2_gcc/bin/activate"
)

config = Config(
    executors=[
        IPyParallelExecutor(
            label='ipp_short',
            provider=slurm,
            nodes=1,
            task_blocks=4,
            init_blocks=4,
            max_blocks=4,
            walltime="00:05:00",
            endpoint=Globus(
                name="7d2dc622-2edb-11e8-b8be-0ac6873fc732",
                path="/home/janedoe/short",
                working_dir="/home/janedoe/short"
            )
        ),
        IPyParallelExecutor(
            label='ipp_long',
            provider=slurm,
            nodes=1,
            task_blocks=4,
            init_blocks=4
            walltime="00:60:00",
            endpoint=Globus(
                name="ddb59aef-6d04-11e5-ba46-22000b92c6ec",
                path="/home/janedoe/long",
                working_dir="/home/janedoe/long"
            )
        ),
        ThreadPoolExecutor(
            label='local_threads',
            max_threads=8
        )
    ],
    lazy_errors=True,
    controller=Controller(public_ip=os.environ.get('PUBLIC_IP')
)

Here is a preview of the Config documentation:

screen shot 2018-05-15 at 1 53 08 pm

The big advantage here is that by starting in the Config documentation, you can click around and see the documentation for all possible configs. In other words, the Config documentation specifies you need an executor, you can click IPyParallelExecutor within that doc to get to the needed specification, etc. Also, if someone hands you this config as a starting point, you can always run

>>>help(SSHChannel)

interactively on various components to reach the documentation (unlike currently, where there's no direct connection between the pieces and the docs).

The proposal above is based entirely on the current code structure. We could always change the underlying organization if needed. @kylechard had some reservations about pool not appearing in here anywhere. Talking with @yadudoc, we do not find a natural place to put it based on the current organization. Feedback is appreciated on this.

@lukaszlacinski

This comment has been minimized.

Copy link
Collaborator

lukaszlacinski commented May 15, 2018

There is no relation between an executor and a Globus endpoint, so an information about a Globus endpoint should not be a part of executors configuration. A Globus endpoint is related to a site by providing a way to access a site filesystem.

@annawoodard

This comment has been minimized.

Copy link
Collaborator Author

annawoodard commented May 15, 2018

To summarize a chat on Slack with @lukaszlacinski and friends: this proposal is dispensing with the term site, and the structure is not changing from what is currently in place. There was a lot of related discussion which @yadudoc will open in a separate issue.

annawoodard added a commit that referenced this issue Jun 7, 2018

Implement class-based configurations
Fixes (partially) Parsl/parsl/#133.

annawoodard added a commit that referenced this issue Jun 8, 2018

Implement class-based configurations
Fixes (partially) Parsl/parsl/#133.

annawoodard added a commit that referenced this issue Jun 12, 2018

Implement class-based configurations
Fixes (partially) Parsl/parsl/#133.

annawoodard added a commit that referenced this issue Jun 12, 2018

Implement class-based configurations
Fixes (partially) Parsl/parsl/#133.

annawoodard added a commit that referenced this issue Jun 18, 2018

annawoodard added a commit that referenced this issue Jun 25, 2018

yadudoc added a commit that referenced this issue Jun 27, 2018

@yadudoc yadudoc closed this in #335 Jul 3, 2018

yadudoc added a commit that referenced this issue Jul 3, 2018

benclifford pushed a commit that referenced this issue Aug 9, 2018

annawoodard added a commit that referenced this issue Sep 24, 2018

Implement class-based configurations
Note this makes the 'channels_required' attribute
unnecessary, so I have removed it. Fixes (partially)
Parsl/parsl/#133.

annawoodard pushed a commit that referenced this issue Sep 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.