Skip to content

Conversation

@masahi
Copy link
Member

@masahi masahi commented Sep 6, 2022

Motivation: Currently, StructuralEqual/Hash take into account NDArray raw data to determine equality / compute hash. This is problematic for link-params = True case, because meta schedule task extraction and database look up will treat the following subgraphs as distinct, even though the only difference is the content of NDArray constants.

def @fused_nn.conv2d_add(%p0: Tensor[(1, 32, 32, 256), float32] /* ty=Tensor[(1, 32, 32, 256), float32] */, Primitive=1) -> Tensor[(1, 32, 32, 256), fl
oat32] {                                                                                                                      
  %0 = nn.conv2d(%p0, meta[relay.Constant][0] /* ty=Tensor[(3, 3, 256, 256), float32] */, padding=[1, 1, 1, 1], channels=256, kernel_size=[3, 3], data_
layout="NHWC", kernel_layout="HWIO") /* ty=Tensor[(1, 32, 32, 256), float32] */;                                                                       
  add(%0, meta[relay.Constant][1] /* ty=Tensor[(1, 1, 1, 256), float32] */) /* ty=Tensor[(1, 32, 32, 256), float32] */        
}

def @fused_nn.conv2d_add(%p0: Tensor[(1, 32, 32, 256), float32] /* ty=Tensor[(1, 32, 32, 256), float32] */, Primitive=1) -> Tensor[(1, 32, 32, 256), fl
oat32] {                                                                                                                      
  %0 = nn.conv2d(%p0, meta[relay.Constant][0] /* ty=Tensor[(3, 3, 256, 256), float32] */, padding=[1, 1, 1, 1], channels=256, kernel_size=[3, 3], data_
layout="NHWC", kernel_layout="HWIO") /* ty=Tensor[(1, 32, 32, 256), float32] */;                                                                       
  add(%0, meta[relay.Constant][1] /* ty=Tensor[(1, 1, 1, 256), float32] */) /* ty=Tensor[(1, 32, 32, 256), float32] */  
} 

This PR adds options to StructuralEqual/Hash to allow ignoring NDArray raw data when comparing / hashing. MS task extraction and database look up are now using these options, since NDArray raw data doesn't matter to them.

cc @Hzfengsy @junrushao1994

@github-actions github-actions bot requested a review from Hzfengsy September 6, 2022 01:22
@tqchen
Copy link
Member

tqchen commented Sep 6, 2022

thanks @masahi . I thin this belongs to a more general topic of canonicalizing the workload keys. While adding such option would indeed resolve this particular case, it would be good to think about if we can have a more generalized approach(for future cases that needs canonicalization, e.g. those that have followup elem-wise patterns which allows some form of reuse)

One possible approach is to have a CanonicalizeWorkloadKey which takes a TIR and Canonicalizes that into a different one(e.g. change alloc_const to a the function parameter which contains a buffer). Depends on how fast we can do this, we can choose to introduce the flag.

This also helps us to prevent possible future cases that can choose to schedule transform depending on the const value content

@masahi
Copy link
Member Author

masahi commented Sep 6, 2022

@tqchen Thanks, I like the idea of the canonicalized key.

Last week I discussed with @zxybazh on the possibility of identifying two subgraphs with the identical anchor op workload (conv2d -> add and conv2d -> add -> add etc) as being equal, to reduce the tuning time like you suggested. I can imagine that, moving away from using the raw Relay / TIR mod as a workload key is a good direction toward that goal.

@masahi masahi force-pushed the structural-ndarray-shallow branch from df0639e to 8c5ef42 Compare September 10, 2022 03:54
junrushao pushed a commit that referenced this pull request Oct 10, 2022
…13001)

When we use `StructuralEqual/Hash`, the actual equality testing / hashing work are deferred to `RemapVarSEqualHandler` and `VarCountingSHashHandler` class respectively, defined in `.cc` files. To customize equality / hash behavior, I want to be able to derive from these classes outside of `structural_equal.cc` or `structural_hash.cc`. This is the first step toward replacing my previous attempt #12706 to allow ignoring NDArray raw data in `StructuralEqual/Hash`.

So I propose to expose them as the "default" handler, while still hiding their implementation details. This lets me define a custom hasher that ignores NDArray content simply by:

```
class SHashHandlerIgnoreNDArray : public SHashHandlerDefault {
 protected:
  void DispatchSHash(const ObjectRef& object, bool map_free_vars) override {
    ICHECK(object.defined());
    if (auto ndarray = object.as<runtime::NDArray::Container>()) {
      SHashReducer hash_reduce(this, map_free_vars);
      NDArrayHash(ndarray, &hash_reduce, false);
    } else {
      SHashHandlerDefault::DispatchSHash(object, map_free_vars);
    }
  }
};
```
junrushao pushed a commit that referenced this pull request Oct 17, 2022
Currently, MS uses `StructuralEqual/Hash` in task extraction / evo search / database. Sometimes, we want to use different hashing and equality testing methods, for example (1) to ignore NDArray (#12706) or (2) to enable anchor-op only tuning (identify `conv2d` and `conv2d -> add` subgraphs as equal). 

To enable such flexibility, this PR consolidate raw calls to `StructuralEqual/Hash` into one place, which for now is named `ModuleEquality`. Since hashing is also done for equality testing, I think it is appropriate to call the component responsible for hashing / equality test that way. But other suggestions are welcome.

Importantly, task extraction and database are now using the same hashing / equal method based on TIR mod, while previously task extraction was using a cache key-ed on relay mod.
@masahi
Copy link
Member Author

masahi commented Oct 17, 2022

Superceded by #13091

@masahi masahi closed this Oct 17, 2022
junrushao pushed a commit that referenced this pull request Oct 17, 2022
…ng NDArray raw data (#13091)

A follow up to #13050, also builds on #13001. This PR enables the functionality in #12706 without changing the existing `StructuralEqual/Hash`.

A question for discussion: Should this be the default ModuleEquality used by MS? It has no effect for the `link-params = False` case, and it simplifies the MS tuning API usage for the `link-params = True` case (Hexagon etc).
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 10, 2022
…e#13050)

Currently, MS uses `StructuralEqual/Hash` in task extraction / evo search / database. Sometimes, we want to use different hashing and equality testing methods, for example (1) to ignore NDArray (apache#12706) or (2) to enable anchor-op only tuning (identify `conv2d` and `conv2d -> add` subgraphs as equal). 

To enable such flexibility, this PR consolidate raw calls to `StructuralEqual/Hash` into one place, which for now is named `ModuleEquality`. Since hashing is also done for equality testing, I think it is appropriate to call the component responsible for hashing / equality test that way. But other suggestions are welcome.

Importantly, task extraction and database are now using the same hashing / equal method based on TIR mod, while previously task extraction was using a cache key-ed on relay mod.
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 10, 2022
…ng NDArray raw data (apache#13091)

A follow up to apache#13050, also builds on apache#13001. This PR enables the functionality in apache#12706 without changing the existing `StructuralEqual/Hash`.

A question for discussion: Should this be the default ModuleEquality used by MS? It has no effect for the `link-params = False` case, and it simplifies the MS tuning API usage for the `link-params = True` case (Hexagon etc).
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
…pache#13001)

When we use `StructuralEqual/Hash`, the actual equality testing / hashing work are deferred to `RemapVarSEqualHandler` and `VarCountingSHashHandler` class respectively, defined in `.cc` files. To customize equality / hash behavior, I want to be able to derive from these classes outside of `structural_equal.cc` or `structural_hash.cc`. This is the first step toward replacing my previous attempt apache#12706 to allow ignoring NDArray raw data in `StructuralEqual/Hash`.

So I propose to expose them as the "default" handler, while still hiding their implementation details. This lets me define a custom hasher that ignores NDArray content simply by:

```
class SHashHandlerIgnoreNDArray : public SHashHandlerDefault {
 protected:
  void DispatchSHash(const ObjectRef& object, bool map_free_vars) override {
    ICHECK(object.defined());
    if (auto ndarray = object.as<runtime::NDArray::Container>()) {
      SHashReducer hash_reduce(this, map_free_vars);
      NDArrayHash(ndarray, &hash_reduce, false);
    } else {
      SHashHandlerDefault::DispatchSHash(object, map_free_vars);
    }
  }
};
```
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
…e#13050)

Currently, MS uses `StructuralEqual/Hash` in task extraction / evo search / database. Sometimes, we want to use different hashing and equality testing methods, for example (1) to ignore NDArray (apache#12706) or (2) to enable anchor-op only tuning (identify `conv2d` and `conv2d -> add` subgraphs as equal). 

To enable such flexibility, this PR consolidate raw calls to `StructuralEqual/Hash` into one place, which for now is named `ModuleEquality`. Since hashing is also done for equality testing, I think it is appropriate to call the component responsible for hashing / equality test that way. But other suggestions are welcome.

Importantly, task extraction and database are now using the same hashing / equal method based on TIR mod, while previously task extraction was using a cache key-ed on relay mod.
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
…ng NDArray raw data (apache#13091)

A follow up to apache#13050, also builds on apache#13001. This PR enables the functionality in apache#12706 without changing the existing `StructuralEqual/Hash`.

A question for discussion: Should this be the default ModuleEquality used by MS? It has no effect for the `link-params = False` case, and it simplifies the MS tuning API usage for the `link-params = True` case (Hexagon etc).
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.

2 participants