-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-2085] Add capability to declare resource hints in Python pipeline transforms. #14390
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
Merged
Merged
Changes from all commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
3d4f386
Add resource hints property to Beam Environments.
tvalentyn 837feb8
Add a mechanism to parse resource hints from command-line options. Se…
tvalentyn d82c5f5
Add a default environment and its urn.
tvalentyn 4333e79
fixup: Explicitly pass desired environment in unit tests to avoid pic…
tvalentyn 6bc3324
Fixup: mypy
tvalentyn 036fe6b
Define resource hints on Beam transforms.
tvalentyn 0b42ca9
Fix a bug where we pass a label instead of object when generating com…
tvalentyn be9f688
to_runner_api translation attaches transforms to environments with re…
tvalentyn ce1d76b
Don't store references to environments in from_runner_api-created obj…
tvalentyn 80b8e53
Pass resource hints through v1beta3 API.
tvalentyn f787688
fixup: mypy
tvalentyn 31291d0
fixup: lint
tvalentyn 4371bac
fixup: add hint parser for 'any' hint, remove leftover code.
tvalentyn 52b7ae0
Propagate hints defined on compostite transforms to subtransforms.
tvalentyn 8d32b96
fixup: lint
tvalentyn ad33440
fixup: minor cleanup.
tvalentyn 6b592c2
Adds tentative resource hints to be recognized by Dataflow Runner.
tvalentyn 1a99ceb
Add Dataflow resource hints translation test.
tvalentyn bb4adbf
Test that environments with same resource hints are reused.
tvalentyn 0f98fc3
Remove test scenarios covered in resourses_test.py.
tvalentyn 0eee01e
Fixup : typo, also use a different hint name to avoid different hint …
tvalentyn 3bd2934
Prefer lazy initialization of (empty) resource hints.
tvalentyn 59fe217
Fixup: mypy
tvalentyn 54e9243
Fixup: style.
tvalentyn 5914c90
Fixup: Use [] as default value for command-line flag.
tvalentyn 9a67adb
Use ResourceHint subclasses to define hint behavior.
tvalentyn cd9da90
fixup: Fix test.
tvalentyn 46e128b
Copy hints onto AppliedPTransform and use them as SOT during translat…
tvalentyn 68b5082
Attach option-specified hints to the root transform.
tvalentyn 5d58030
Propagate hints when AppliedPTransforms are attached to pipeline grap…
tvalentyn 0477bc5
Fixup: isort
tvalentyn cc0e3ef
fix bug/typo in Dataflow SideInputVisitor
tvalentyn 0d6e62e
fixup: use Pipeline instead of TestPipeline
tvalentyn f2268f0
Fixup: Removes super() delegations that can be omitted.
tvalentyn 589e263
Fixup: documentation on proto.
tvalentyn 3d0f669
Prevent hash collisions on environments with same hints. Also compare…
tvalentyn 63d8594
fixup: nits.
tvalentyn 1fdadb8
fixup: nits.
tvalentyn 97326d7
fixup f787: Instantiate Environment directly.
tvalentyn 2176c43
fixup: Don't use fake coders.
tvalentyn 95fd0c2
Return a fresh copy of hints dictionary when merging hints.
tvalentyn c6d59df
Make resource hint application additive.
tvalentyn fe56eca
fixup (0b42): Restrict get_by_proto to Environment protos only.
tvalentyn d7b2d83
fixup (c6d5): isort
tvalentyn bf81698
Allow alternative spelling minRam for memory hint.
tvalentyn c14a06a
Prevent users from accidentally supplying a min_ram hint on the comma…
tvalentyn a245612
fixup (bb4ad): Add an extra assertion to ensure scenario correctness.
tvalentyn 571be41
Merge master into resource hints feature branch.
tvalentyn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
== 1? Or at least > 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
num_iter is 4
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't hurt to assert that it is > 1 though.
FWIW while working on this test I noticed that the to-from translation is somewhat slow, even without my changes. This test took almost 2 seconds to run on my laptop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's slow. I don't think much time has gone into optimizing it, but speeding it up would especially help TFX pipelines (that tend to have lots and lots of stages).