Next: Refactor internals to support aggregation, improved performance and documentation. #427
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes the submission id generation to prepare for aggregation. The `__str__` method of `_JobOperation` is also changed to better handle job aggregation. The unique id generated using a hash of all the `_JobOperation` object's data is still the same, so the uniqueness should not be in question; the only change is to the readable part of the id. The change to the `str` implementation mimics NumPy, and the submission id favors brevity.
Add a new decorator by which operations can be tagged for various types of aggregation, along with classes to store actual aggregates once they have been generated.
…ing (#351) * Use sort_ascending instead of reverse_order. * Fix and simplify hash methods. * Clarify docstrings. * Initialize list/dict with literals. * Use one-line function instead of defining a separate method. * Don't wrap line. * class -> object. * use itertools reference while using groupby * Use reference for zip_longest too * Document _get_unique_function_id Co-authored-by: Hardik Ojha <[email protected]>
…erlab/signac-flow into feature/enable-aggregation
…erlab/signac-flow into feature/enable-aggregation
Change the internals of flow so that everything operates on default aggregates (aggregates of size 1) rather than individual jobs.
Feature/enable aggregation
* Remove unused __init__, the implicit parent constructor is equivalent. * Use public .job attribute. * Remove unused argument from _add_operation_selection_arg_group. * Use logger.warning instead of deprecated logger.warn.
…led (it's a hard error).
* Deprecate CPUEnvironment and GPUEnvironment. The classes are unused and untested. * Remove unused private functions for importing from files. * Update changelog.
Introduces stylistic changes suggested by pylint.
* Refactor dicts, OrderedDict, and comprehensions. Resolves #323. * Import Mapping from collections.abc. * Use {}
* Use longer/clearer variable names. * Update _no_aggregation. * Rename _operation_to/from_tuple to _job_operation_to/from_tuple Co-authored-by: Brandon Butler <[email protected]>
* Make _to_hashable private. * Copy _to_hashable method from signac.
* Add and improve docstrings. * Minor edits to docstrings. * Use shorter one-line summaries in some long docstrings.
* Clean up docs and variables relating to pre-conditions and post-conditions. * Clean up docstrings, variable names, defaults, remove obsolete helper functions. * Use f-strings, avoid second-person "you". * Use full name for template_filters. * Docstring revisions.
Validates docstrings using pydocstyle and adds all the necessary documentation for this to pass.
* Remove the use of argument * Remove the use of env attribute while submit * Pass environment as keyword argument. * Remove env argument in a few missed places. * Fix template reference data. It was using mpiexec (default) and should use the cluster-specific MPI commands. I don't know why the reference data was wrong. * Remove extra comment about status parallelization. Co-authored-by: Bradley Dice <[email protected]>
* Always buffer, add deprecation warning if use_buffered_mode=False. * Remove unbuffered tests. * Reduce number of jobs to speed up tests. * Update changelog. * Update deprecation notice.
* Refactor status rendering into a private function. Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]> * Renamed fakescheduler to fake_scheduler. Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]> * Make environment metaclass private. Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]> * Remove unused status.py file. Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]> * Remove out argument from templates. Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]> * Refactor scheduler classes, unify code. Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]> * Remove metaclass from docs. Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]> * Update docstrings. * Update docstrings. * Update changelog. * Update docstrings. * Update changelog.txt * Update changelog.txt Co-authored-by: Alyssa Travitz <[email protected]> Co-authored-by: Hardik Ojha <[email protected]>
bdice
changed the title
bdice
changed the title
Cleans up various internals associated with aggregation, introducing additional abstraction layers and adding docstrings. * Refactor aggregation. * Fix failing tests from refactor. * Improve clarity and coverage of tests. * Add more tests to enforce that aggregates are tuples. * Expand tests of default aggregator. * Update tests. * Unify __iter__. * __getitem__ should raise a KeyError if not found. * Update error message. * Update flow/aggregates.py Co-authored-by: Vyas Ramasubramani <[email protected]>
Rewrite the _fetch_status method, which was extremely large and unwieldy. The new code is more efficient since it avoids making multiple passes through the jobs. It also uses some newly introduced functions and context managers as well as newer tqdm functions to clean up existing code. * Initial rewrite of _fetch_status. * Update _fetch_scheduler_status to require aggregates. * Clarify docstring for status_parallelization. * Experiment with callback approach (only iterate over aggregates/groups once). * Skip evaluation of post-conditions during eligibility if known to be incomplete. * Change operations to groups. * Use context manager for updating the cached status in bulk. * Separate scheduler query from cached status update. * Fix docstring. * Use context manager for _fetch_scheduler_status cache update. * Use cached status context manager for _fetch_status. * Refactor status updates so the scheduler query happens on-the-fly and only considers selected groups/aggregates. * Intermediate commit with notes. * Simplify eligibility check. * Update docstring. * Update tests to account for removed method. * Rename cid to cluster_id. * Define parallel executor. * Only update status if the update dictionary has data. * Add parallel execution. * Remove status callback. * Clean up status fetching. * Remove unnecessary internal methods. * Fix test (scheduler is queried internally). * Rename jobs -> aggregate. * Fix bug in tests (the dynamically altered jobs should NOT be resubmitted, this was probably an error due to the use of cached status rather than querying the scheduler). * Show aggregate id in str of _JobOperation. * Fix script output. * Remove keyword argument. * Reset MockScheduler in setUp for all tests. * Mock root directory (can't override Project.root_directory() because it is used for all job document paths and buffering, and must reflect an actual path on disk). * Update template reference data. * Refactor mocked root. * Pickle function to be executed in parallel using cloudpickle so that it can be sent across threads. * Fix pre-commit hooks. * Fix root directory mocking. * Improve progress iterators. * Use chunked job label fetching. * Refactor parallel executor to accept one iterable and use Pool for process parallelism. * Use integers in the cached status update. * Fix mocking of system executable. Resolves #413. * Update changelog. * Mock environment directly rather than using **kwargs for compatibility with signac 1.0.0. * Buffer during project status update. * Use ordered results. * Don't buffer during status update (no performance difference). * Refactor job labels so that the list of individual jobs is generated during the same single loop over the project to generate aggregates. * Update flow/util/misc.py Co-authored-by: Vyas Ramasubramani <[email protected]> * Fix function parameters in test, use kwargs for _fetch_status. * Use process_map. * Use MOCK_EXECUTABLE. * Add comments explaining use of FakeScheduler. * Collect errors that occur during status evaluation. * Mock the id generation method instead of injecting mocked attributes. Co-authored-by: Vyas Ramasubramani <[email protected]> Co-authored-by: Vyas Ramasubramani <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #427 +/- ##
==========================================
+ Coverage 66.85% 70.44% +3.58%
==========================================
Files 29 29
Lines 2794 2994 +200
Branches 496 553 +57
==========================================
+ Hits 1868 2109 +241
+ Misses 795 737 -58
- Partials 131 148 +17
Continue to review full report at Codecov.
|
* Make aggregator and get_aggregate_id private * Remove reference from docs * Use private name for _aggregator in test_aggregates.py. * Remove aggregates from documentation, add TODOs. * Fix docstring reference. * Update changelog. Co-authored-by: Bradley Dice <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR merges the
nextbranch. This branch refactors the internals of signac-flow to support aggregation (execution of operations/groups on multiple jobs at once). Thenextbranch contains the following high-level changes:To-do before merging:
make_groupas needed (done in Make class aggregator and method get_aggregate_id private #432)Committers: this PR should NOT be squashed when merging.
After this PR is merged:
Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist:
If necessary: