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

Implement get_pcg_series_parallel_decomposition #1598

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

lockshaw
Copy link
Collaborator

@lockshaw lockshaw commented Feb 16, 2025

Description of changes:

  • Implement get_pcg_series_parallel_decomposition
  • Move initializers from TensorAttrs to WeightAttrs
  • Remove ParamSync for now
  • Use shape inference in add_layer functions for ComputationGraph and ParallelComputationGraph
  • Add a TensorShape field in InputAttrs, forcing all InputAttrs to be non-parallelized for now
  • Implement default initializers to match pytorch for a number of layers

Related Issues:

Linked Issues:

  • Issue #

Issues closed by this PR:


This change is Reviewable

@lockshaw lockshaw requested review from Marsella8 and wmdi February 16, 2025 02:48
Copy link

codecov bot commented Feb 16, 2025

Codecov Report

Attention: Patch coverage is 70.41885% with 226 lines in your changes missing coverage. Please review.

Project coverage is 62.90%. Comparing base (e9a1af7) to head (3d9dee4).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
lib/op-attrs/src/op-attrs/shape_inference.cc 74.16% 54 Missing ⚠️
...el_computation_graph/parallel_computation_graph.cc 53.33% 28 Missing ⚠️
lib/op-attrs/src/op-attrs/initializer_attrs.cc 0.00% 25 Missing ⚠️
lib/local-execution/src/local_cost_estimator.cc 0.00% 14 Missing ⚠️
lib/op-attrs/src/op-attrs/ops/batch_norm.cc 47.61% 11 Missing ⚠️
lib/op-attrs/src/op-attrs/ops/repartition.cc 0.00% 11 Missing ⚠️
lib/op-attrs/src/op-attrs/ops/attention.cc 83.05% 10 Missing ⚠️
lib/op-attrs/src/op-attrs/ops/combine.cc 0.00% 10 Missing ⚠️
lib/op-attrs/src/op-attrs/ops/reduction.cc 0.00% 10 Missing ⚠️
lib/op-attrs/src/op-attrs/ops/replicate.cc 0.00% 10 Missing ⚠️
... and 13 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1598      +/-   ##
==========================================
+ Coverage   62.86%   62.90%   +0.04%     
==========================================
  Files         632      635       +3     
  Lines       14804    15201     +397     
==========================================
+ Hits         9306     9562     +256     
- Misses       5498     5639     +141     
Flag Coverage Δ
unittests 62.90% <70.41%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...computation_graph_series_parallel_decomposition.cc 100.00% <100.00%> (ø)
...allel/pcg/get_pcg_series_parallel_decomposition.cc 100.00% <100.00%> (+100.00%) ⬆️
lib/local-execution/src/local_slots_backing.cc 83.16% <100.00%> (ø)
lib/local-execution/src/ops/batch_matmul.cc 0.00% <ø> (ø)
lib/local-execution/src/ops/concat.cc 0.00% <ø> (ø)
lib/local-execution/src/ops/conv_2d.cc 0.00% <ø> (ø)
lib/local-execution/src/ops/dropout.cc 0.00% <ø> (ø)
lib/local-execution/src/ops/element_binary.cc 0.00% <ø> (ø)
lib/local-execution/src/ops/element_unary.cc 0.00% <ø> (ø)
lib/local-execution/src/ops/flat.cc 0.00% <ø> (ø)
... and 50 more

... and 5 files with indirect coverage changes

Copy link
Collaborator

@wmdi wmdi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 128 of 128 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Marsella8)


lib/compiler/test/src/compiler/machine_mapping/get_tensor_set_movement_across_split.cc line 13 at r1 (raw file):

using namespace ::FlexFlow;

bool isDebuggerActive() {

Do you want to move it to somewhere under utils? or if it was just used temporarily, just remove it?

@lockshaw lockshaw enabled auto-merge (squash) February 20, 2025 02:13
Copy link
Collaborator

@wmdi wmdi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lockshaw and @Marsella8)

@lockshaw lockshaw merged commit 10ef31b into flexflow:master Feb 20, 2025
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add weight handling for SP decomposition of PCGs
2 participants