You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/03/02 15:38:33 UTC
[GitHub] [incubator-tvm] deepakbabel opened a new pull request #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
deepakbabel opened a new pull request #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979
Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] deepakbabel commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
deepakbabel commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595735969
Hi @MarisaKirisame
I understand now that in the new design proposed by you, we want to enforce unique random sequences every-time even if the input seed provided by user is same between multiple session executions. We are internally using C++11 std::mt19937 generator for PRNG generation. There are no such API available in this generator which returns a new random seed value along with the output RNG object. Please see "src/runtime/contrib/random/mt_random_engine.cc" for more details.
In such case how do you propose getting the new random seed. Also this piece of tvm code for PRNG generation(mt_random_engine.cc, random.cc) was pre-existing in contrib layer, we have just provided support in relay and topi layers so that it can be called by both these layers.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] MarisaKirisame commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-593471655
@jroesch @tqchen Having effect in the TVM ir will make optimization on that level harder (because now it has to do effect analysis), and effect do not play nice with Relay's graph representation. I personally think that all TVM operator should be referential transparent, and the effect should happen at the relay level.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] MarisaKirisame commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-594156931
@tqchen I agree that at some level we should provide a stateful api. I think it will be best provided in relay. For example, there will be a global mutable reference holding a seed, and there are relay wrapper that call the pure tvm operator with the old seed, and store the new seed.
Or one could think about a relay pass that transform stuff into PRNG passing style.
@deepakbabel the function should at least output the new seed then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] deepakbabel commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
deepakbabel commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595740583
Even if we use tf, we get the same sequence even if we use global and operation seeds in conjunction across multiple session calls
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] MarisaKirisame commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-593467985
this has unnecessary effect (nondeterminism).
can you make it input a random seed and output a random seed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] deepakbabel edited a comment on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
deepakbabel edited a comment on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595735969
Hi @MarisaKirisame
I understand now that in the new design proposed by you, we want to enforce unique random sequences every-time even if the input seed provided by user is same between multiple session executions. We are internally using C++11 std::mt19937 generator for PRNG generation. There are no such API available in this generator which returns a new random seed value along with the output RNG object. Please see "src/runtime/contrib/random/mt_random_engine.cc" for more details.
In such case how do you propose getting the new random seed. Also this piece of tvm code for PRNG generation(mt_random_engine.cc, random.cc) was pre-existing in contrib layer, we have just provided support in relay and topi layers so that it can be called by both these layers.
Hope you are not referring to getting back the current seed(user provided seed). If so, then can use GetSeed() method of our RandomEngine class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] deepakbabel edited a comment on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
deepakbabel edited a comment on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595740583
@MarisaKirisame : By the way when I had explored Tensorflow, I had observed that we get the same sequence even when we use the previous seeds(global and operation seeds) again in conjunction across multiple session calls
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] MarisaKirisame commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595330595
I am proposing the following design:
the tvm operator randomuniform take a seed, and return a random uniform tensor paired with the new seed.
relay has a global reference holding a seed.
there are relay helper function that, read the seed, call randomuniform, write the new seed to the reference, and returning the tensor.
this way, the tvm level can be kept pure, which is beneficial for optimization, while the relay level provide effectful api that is easy to use.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-596221752
To chime in a bit here. I think the main thing that need to be discussed is not the implementation, but how can PRNG being represented in the high-level relay graph.
Inherently, the low level program need to have some of the opaque global states in certain execution mode(e.g. IO, PRNG or cudnn handle). It does not, however, prevent us from adopting a functional style program at the high-level. as these functional style program can still be lowered into low-level program that has side-effect(via a global random number generator).
```python
data1, rng = uniform(rng, shape)
data2, rng = uniform(rng, shape)
```
For example, the program above can be lowered by ignoring `rng` as an empty object, but makes use of the data flow dep to preserve ordering of the random numbers(where before they do not have dependencies.
We do however, would like to discussion a few alternatives, and how to best represent such program in relay overall. Functional style program representation is easier to transform, distribute, but also brings interesting challenge about how to construct them initially from program that may not have a functional form. Let us open an RFC to discuss that
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] MarisaKirisame commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595598371
Because if there is only one seed, every random uniform call will return the same result for the same tensor size.
Internally, the random computation proceed already by using a random number genrator and advancing the seed. This is the implementation detail of the PRNG.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-593753111
I think it worth start an RFC discussion thread about how are we going to handle PRNGs in general. In particular, I agree that in the relay API, perhaps it is easier(for optimization) to make the RPNG takes in an RNG object and outputs an RNG object to preserve the ordering.
At the low-level though, I am not sure what is the exact approach we would like to take. At some timepoint we will need to go back to the stateful land where we keep a PRNG state somewhere (in CPU or GPU)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] deepakbabel edited a comment on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
deepakbabel edited a comment on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-593815922
> this has unnecessary effect (nondeterminism).
> can you make it input a random seed and output a random seed?
@MarisaKirisame : Thank you for your inputs. The original intent here was if the user would provide an input seed(as per the user choice) and desired shape of RNG object, then the function would generate the RNG object based on the input seed value. If the user uses the same seed(seed value != 0) again in the same session, he would get the same RNG or PRNG object. Only when seed value == 0, non-determinism kicks in and the RNG object would be different for each run in a given session.
Does it make sense? Let me know if I understood you correctly and the response is on the same lines.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] maheshambule commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
maheshambule commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595320437
@MarisaKirisame, could you please elaborate more on outputting the new seed? Does it mean that in case if seed is not provided Relay should generate (and not the operator to keep it pure) a new seed, pass it to the operator and then store it in a global mutable reference at Relay level. And in case if seed is provided just pass it to the operator.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] maheshambule commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
maheshambule commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-595393925
Thanks @MarisaKirisame. I understand that We want to keep TVM operator pure and Relay can have side effects. But I am not able to figure out Why do we want the operator to return the new seed? And how that new seed will get generated?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] deepakbabel commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
deepakbabel commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-593815922
> this has unnecessary effect (nondeterminism).
> can you make it input a random seed and output a random seed?
Thank you for your inputs. The original intent here was if the user would provide an input seed(as per the user choice) and desired shape of RNG object and the function would generate the RNG object based on the same. If the user uses the same seed(seed value != 0) again in the same session, he would get the same RNG object. Only for seed value == 0, non-determinism kicks in and the RNG object would be different for each run in a given session.
Does it make sense? Let me know if I understood you correctly and the response is on the same lines.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] deepakbabel commented on issue #4979:
[Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
deepakbabel commented on issue #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-593793213
@yongwww , Adding you also to the reviewer list
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen closed pull request #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
tqchen closed pull request #4979:
URL: https://github.com/apache/incubator-tvm/pull/4979
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] JoeyTPChou commented on pull request #4979: [Relay/TOPI/TF][Op] Add support for randomuniform Op in TVM
Posted by GitBox <gi...@apache.org>.
JoeyTPChou commented on pull request #4979:
URL: https://github.com/apache/incubator-tvm/pull/4979#issuecomment-702343916
Hi, just wanna check, is there still a plan to merge this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org