Page MenuHomePhabricator

ahmedsabie (Ahmed Sabie)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 27 2020, 3:50 PM (13 w, 6 d)

Recent Activity

Oct 30 2020

ahmedsabie added a comment to D86747: [MLIR] Add an OpRewritePattern constructor that uses generated names.

Sorry forgot this never actually ended up getting pushed and it's my last day

Oct 30 2020, 12:52 PM · Restricted Project

Oct 27 2020

ahmedsabie added inline comments to D89572: Add element preserving trait to optimize involution folding.
Oct 27 2020, 10:18 AM · Restricted Project

Oct 25 2020

ahmedsabie retitled D89572: Add element preserving trait to optimize involution folding from Add value preserving trait to optimize idempotent folding to Add element preserving trait to optimize involution folding.
Oct 25 2020, 8:26 PM · Restricted Project

Oct 22 2020

ahmedsabie added a comment to D89572: Add element preserving trait to optimize involution folding.

I don't really understand the motivation here, can you detail it a bit more. Why is the original trait not enough? Also, ValuePreserving seems too broad of a name for what this is doing.

This makes the original trait applicable in more places because it can ignore nodes that don't actually end up changing tensor values so you can still fold.

I don't understand this aspect. Is this supposed to mean that the operation would somehow mutate the input tensors? Putting the fact that tensors are value-typed aside, I would expect this to be something covered by side effects and not a trait like this. If an operation is going to mutate its input operands, that is something that should be expressed as a side effect.

No I mean if you had abs(permute(abs(x))) then that's the same as permute(abs(x)) but you can't generally apply this optimization to all side effect free operations, example abs(negate(abs(x))) is not the same as negate(abs(x)). ValuePreserving is capturing the property of permute that it doesn't actually produce new values when applied

Thank you for explaining, I understand what you mean by value preserving now. That being said, I don't think this optimization is legal with just idempotent and "value preserving". For example, stable_sort(permute(stable_sort(x))) is not strictly the same as permute(stable_sort(x)).

Oct 22 2020, 12:17 PM · Restricted Project

Oct 21 2020

ahmedsabie updated the diff for D89572: Add element preserving trait to optimize involution folding.

Update comment

Oct 21 2020, 10:12 AM · Restricted Project

Oct 20 2020

ahmedsabie updated the diff for D89572: Add element preserving trait to optimize involution folding.

Update name of trait and apply it to involutions instead

Oct 20 2020, 2:33 PM · Restricted Project

Oct 18 2020

ahmedsabie added a comment to D89572: Add element preserving trait to optimize involution folding.

I don't really understand the motivation here, can you detail it a bit more. Why is the original trait not enough? Also, ValuePreserving seems too broad of a name for what this is doing.

This makes the original trait applicable in more places because it can ignore nodes that don't actually end up changing tensor values so you can still fold.

I don't understand this aspect. Is this supposed to mean that the operation would somehow mutate the input tensors? Putting the fact that tensors are value-typed aside, I would expect this to be something covered by side effects and not a trait like this. If an operation is going to mutate its input operands, that is something that should be expressed as a side effect.
No I mean if you had abs(permute(abs(x))) then that's the same as permute(abs(x)) but you can't generally apply this optimization to all side effect free operations, example abs(negate(abs(x))) is not the same as negate(abs(x)). ValuePreserving is capturing the property of permute that it doesn't actually produce new values when applied

Oct 18 2020, 8:50 PM · Restricted Project
ahmedsabie added a comment to D89572: Add element preserving trait to optimize involution folding.

I don't really understand the motivation here, can you detail it a bit more. Why is the original trait not enough? Also, ValuePreserving seems too broad of a name for what this is doing.

Oct 18 2020, 8:07 PM · Restricted Project

Oct 16 2020

ahmedsabie requested review of D89572: Add element preserving trait to optimize involution folding.
Oct 16 2020, 11:12 AM · Restricted Project
ahmedsabie added a comment to D89421: [MLIR] Add idempotent trait folding.

Can you push it when you get a chance I don't have push access, thanks!

Oct 16 2020, 8:34 AM · Restricted Project

Oct 14 2020

ahmedsabie added reviewers for D89421: [MLIR] Add idempotent trait folding: andyly, Smit, mehdi_amini.
Oct 14 2020, 1:06 PM · Restricted Project
ahmedsabie requested review of D89421: [MLIR] Add idempotent trait folding.
Oct 14 2020, 1:05 PM · Restricted Project

Oct 13 2020

ahmedsabie added a comment to D89333: [MLIR] Add back trait based folding and add FIXME message for adding side effect free check to Involution.

I dont have push access, if you get a chance can you push for me? Thanks!

Oct 13 2020, 2:12 PM · Restricted Project
ahmedsabie added reviewers for D89333: [MLIR] Add back trait based folding and add FIXME message for adding side effect free check to Involution: mehdi_amini, andyly, Smit.
Oct 13 2020, 11:04 AM · Restricted Project
ahmedsabie retitled D89333: [MLIR] Add back trait based folding and add FIXME message for adding side effect free check to Involution from [MLIR] Add FIXME message for adding back side effect free check to Involution to [MLIR] Add back trait based folding and add FIXME message for adding side effect free check to Involution.
Oct 13 2020, 11:03 AM · Restricted Project
ahmedsabie updated the diff for D89333: [MLIR] Add back trait based folding and add FIXME message for adding side effect free check to Involution.

Add the missing revert

Oct 13 2020, 10:59 AM · Restricted Project
ahmedsabie requested review of D89333: [MLIR] Add back trait based folding and add FIXME message for adding side effect free check to Involution.
Oct 13 2020, 10:58 AM · Restricted Project

Oct 9 2020

ahmedsabie added a comment to D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.

Pushed a revert in 16580d791f3b ; can you try to build after configuring cmake with -DBUILD_SHARED_LIBS=ON ?

Oct 9 2020, 2:33 PM · Restricted Project

Oct 8 2020

ahmedsabie added a comment to D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.

I don't have push access so if you get a chance to push my code and its good please do, thanks!

Oct 8 2020, 2:53 PM · Restricted Project
ahmedsabie updated the diff for D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.

Fix last few minor comments

Oct 8 2020, 2:53 PM · Restricted Project

Oct 7 2020

ahmedsabie added inline comments to D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.
Oct 7 2020, 3:24 PM · Restricted Project
ahmedsabie updated the diff for D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.

Allow traits to continue folding if an inplace fold occured

Oct 7 2020, 3:23 PM · Restricted Project

Oct 6 2020

ahmedsabie added inline comments to D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.
Oct 6 2020, 12:57 PM · Restricted Project
ahmedsabie updated the diff for D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.

Added no side effect requirement to involution, changed fold to two different signatures to be consistent with existing fold functionality, and fixed issues addressed by comments

Oct 6 2020, 12:53 PM · Restricted Project

Oct 5 2020

ahmedsabie added a comment to D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.

I agree with Mehdi on keeping it as one PR but updating the name since making a separate PR with just the trait folding would require writing code for its test that's almost the same as what's required for involution anyways

Oct 5 2020, 9:38 AM · Restricted Project
ahmedsabie retitled D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait from [MLIR] Introduce involution based folding to [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.
Oct 5 2020, 9:36 AM · Restricted Project
ahmedsabie updated the diff for D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.

update commit message to be more descriptive of the infrastructure change

Oct 5 2020, 9:35 AM · Restricted Project

Oct 4 2020

ahmedsabie added reviewers for D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait: mehdi_amini, andyly, Smit.
Oct 4 2020, 9:55 PM · Restricted Project
ahmedsabie requested review of D88809: [MLIR] Add a foldTrait() mechanism to allow traits to define folding and test it with an Involution trait.
Oct 4 2020, 9:55 PM · Restricted Project

Aug 27 2020

ahmedsabie requested review of D86747: [MLIR] Add an OpRewritePattern constructor that uses generated names.
Aug 27 2020, 3:54 PM · Restricted Project