Implementing a grouping feature to organize flow operations

[b-butler]

Description

Goal of this branch is to create a grouping feature that will allow for operations in FlowProject
to be organized into "metaoperations." Currently to impliment this behavior either a new
FlowProject has to operate on the original set of operations or user commenting has to be
done to prevent the group operations from running individually.

An example snippet of expected functional shows in more detail a potential API and use
of the feature,

a_group = Project.make_group('a')

@Pr.operation
@a_group
def foo(job):
    pass

@Pr.operation
@a_group
def bar(job):
    pass

Then to use the grouping a command like,
submit --group a_group
would output a job for the scheduler with the final command being something
akin to
python project.py run -j abc123 -o foo bar

Motivation and Context

This change is a feature change that seeks to give the user more control over the workflow
of their project. The group concept will allow for operations to be tagged to be run at once
in a single job which can work even if the pre and post conditions are not met initially but will
be once other operations have run.

This abstraction of a workflow is useful for multiple scenerios such as automatically restricting
operations to particular environements, running a job repeatedly in a single submit #4 , and moving
workflow logic into the FlowProject and not the individual operations for idiomatic code. Another use
case would to be mark operations as submit only #33 . Another issue tangentially related would be #41 as groups could be considered to create graphs themselves or groups might make up higher
level graphs.

Types of Changes

• Documentation update
• Bug fix
• New feature
• Breaking change¹

¹The change breaks (or has the potential to break) existing functionality.

Checklist:

• I am familiar with the Contributing Guidelines.
• I agree with the terms of the Contributor Agreement.
• My name is on the list of contributors.
• My code follows the code style guideline of this project.
• The changes introduced by this pull request are covered by existing or newly introduced tests.

If necessary:

• I have updated the API documentation as part of the package doc-strings.
• I have created a separate pull request to update the framework documentation on signac-docs and linked it here.
• I have updated the changelog.

[csadorf]

Hey! I've gone through the changes on a superficial level and just added a few comments with questions. I'd appreciate if you could respond to those to clear a few things up for me and then we can move forward from there.

[csadorf]

@b-butler Please ping me here when you want my input, thank's!

[b-butler]

@csadorf I was wondering if you would give the code a look. I believe everything should work now in the way we talked about it working with the main function. I still have to change the plain submit, script, and run functions.

[csadorf]

I've tested this locally and went through most of the code. Really nice! Works like expected.

I'd appreciate if you could address the comments and suggestions. Thank you!

[csadorf]

@b-butler One additional comment, since you are running into some linter issues, make sure to follow the code code contribution instructions on how to setup a developer environment. In that way you avoid committing any code that will fail the pipeline due to linter issues.

[b-butler]

This commit won't meet the checks either for the style standards. I will set up the correct environment at work tomorrow.

[csadorf]

I was just playing around with this feature and then I wondered: Why are we not defining that the specification of one more group leads to a selection of the union of operations of those groups? Is it because of the directives? At least for the run-command it should be possible, right?

[b-butler]

@csadorf that would be a possible option, but options for "run" mode would be confused at that point. If we had specified run options, they couldn't be honored. Also directives like you said could come into conflict. I don't see much benefit from combining groups this way, but there are a few conflicts it could raise.

What would be the benefits for the user on this approach?

[csadorf]

What would be the benefits for the user on this approach?

Well, it would make it possible to execute a union of multiple operation groups. I interpret the groups more as tags, so I could tag some as workstation, and others with plotting, and they might overlap in some way. How would I be able to execute the union of both groups otherwise?

I understand that this might not be possible for submissions, but I see no downside for local runs.

[b-butler]

The union is already done for local run commands, or at least it is a bug if it isn't.

• I could see an option to submit the union too, but I would think that it may better fit into a command-line argument.
• Another way could be to check to see if options are identical, but since options are currently just strings to append to the run style command, we would need to change the format to a dictionary and create the appending string when __call__() is called. Then the union could be taken conditionally, and if options are different and the intersection of operations is not null then an error is thrown.

I am not sure which would be most useful and least confusing for users though.

[csadorf]

@b-butler Please let me know when I should give it another pass, thx!

[b-butler]

This is the documentation pull request: glotzerlab/signac-docs#36

[b-butler]

I think the code with testing is at a good point now. The only thing not tested is the group after feature which (since it requires correct ordering of decorators) shouldn't probably be exposed to the users just yet.

If we want to support the union of groups for submission as a command-line argument that still has to be implemented and tested.

[csadorf]

@b-butler I've started reviewing and testing this. Could you make sure to either merge or rebase on master?

[csadorf]

I've left some comments and in addition (per offline discussion), each FlowOperation should explicitly be a one-member FlowGroup and FlowGroups should be the only place where we manage directives.

[csadorf]

@b-butler Please ping me here when I should give this another pass. Thank you.

[csadorf]

@b-butler What's the status here? Should I review/test?

[b-butler]

@csadorf yes I would you to test it out to try and break it. I can't seem to get the simple-scheduler working well on my desktop or laptop and couldn't find documentation on it. Forgot you wanted me to ping you, sorry.

[b-butler]

@vyasr and @csadorf I have addressed the additional comments through either discussion on the comment or implementing the necessary changes. Tests now pass besides template tests.

[csadorf]

I tested this locally found one more bug and added a few more action items in the comments, but otherwise I am satisfied enough to approve this for merge for beta-testing! @b-butler Thank you so much for seeing this through, you absolutely deserve an award for this.

[csadorf]

@vyasr I approved this PR pending that the submission template tests are updated, checked, and related tests reactivated.

• Update and review submission templates
• Re-enable submission integration tests

[vyasr]

@b-butler I have a few very small requests/questions. I'd be fine approving, except we also need to get the template tests back. I suggest we find a time where we sit together and address the last couple of items as well as reenabling template tests, then I'll approve and we can get this merged!

[vyasr]

Fantastic work! We'll make incremental edits as we beta-test, but I'm ready to merge. @b-butler you can do the honors once tests pass.