Skip to content

Reduction OP#1120

Merged
reyna-abhyankar merged 13 commits intoflexflow:repo-refactorfrom
lambda7xx:repo-refactor-lambda-Reduction-OP
Nov 2, 2023
Merged

Reduction OP#1120
reyna-abhyankar merged 13 commits intoflexflow:repo-refactorfrom
lambda7xx:repo-refactor-lambda-Reduction-OP

Conversation

@lambda7xx
Copy link
Contributor

@lambda7xx lambda7xx commented Sep 8, 2023

Description of changes:

  • upate the Reduction OP

Related Issues:

Linked Issues:

Issues closed by this PR:


This change is Reviewable

@lambda7xx lambda7xx self-assigned this Sep 8, 2023
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: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @lambda7xx, @lockshaw, and @reyna-abhyankar)


lib/runtime/src/ops/reduction.cc line 19 at r1 (raw file):

#include "kernels/reduction_kernels.h"
#include "op-attrs/get_output_shape.h"
#include "utils/exception.decl.h"

decl.h files should not be included here.

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 2 of 3 files at r1, all commit messages.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @lambda7xx, @lockshaw, and @reyna-abhyankar)


lib/runtime/src/ops/reduction.cc line 19 at r1 (raw file):

Previously, wmdi (Mengdi Wu) wrote…

decl.h files should not be included here.

I also noticed a few other files outside utils including.decl.h. Remember to update them as well.

Copy link
Contributor Author

@lambda7xx lambda7xx 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: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @lockshaw, @reyna-abhyankar, and @wmdi)


lib/runtime/src/ops/reduction.cc line 19 at r1 (raw file):

Previously, wmdi (Mengdi Wu) wrote…

I also noticed a few other files outside utils including.decl.h. Remember to update them as well.

Done.

@lambda7xx
Copy link
Contributor Author

lib/runtime/src/ops/reduction.cc line 19 at r1 (raw file):

Previously, lambda7xx (Lambda(Xiaoxiang) Shi ) wrote…

Done.

currently, other files like contaners.h use .decl.h is not my side. I will check my other pr

@lockshaw lockshaw removed their request for review October 7, 2023 00:11
Copy link
Contributor Author

@lambda7xx lambda7xx 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: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @lockshaw, @reyna-abhyankar, and @wmdi)


lib/runtime/src/ops/reduction.cc line 79 at r2 (raw file):

Previously, reyna-abhyankar (Reyna Abhyankar) wrote…

bump @lockshaw

Done.


lib/runtime/src/ops/reduction.cc line 149 at r2 (raw file):

Previously, reyna-abhyankar (Reyna Abhyankar) wrote…

bump @lambda7xx

Done.

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 all commit messages.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @lambda7xx, @lockshaw, and @reyna-abhyankar)

wmdi
wmdi previously approved these changes Oct 18, 2023
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 1 of 1 files at r4.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lockshaw and @reyna-abhyankar)

Copy link
Contributor Author

@lambda7xx lambda7xx 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, 2 unresolved discussions (waiting on @reyna-abhyankar)


lib/runtime/src/ops/reduction.cc line 79 at r2 (raw file):

Previously, lockshaw (Colin Unger) wrote…

Yeah I agree @reyna-abhyankar

Done.

@reyna-abhyankar reyna-abhyankar enabled auto-merge (squash) October 26, 2023 01:09
@lambda7xx lambda7xx requested review from lockshaw and wmdi October 26, 2023 01:11
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.

Update Reduction operator

4 participants