Re: LSTM regression (was RE: Include MKLDNN into default mxnet pip package)
Ran benchmark and it addresses issue. Thanks. On 11/28/18, 6:02 PM, "Zhao, Patric" wrote: MKL-DNN v0.17.1 is released https://github.com/intel/mkl-dnn/tree/v0.17.1 I have submitted the PR to pin this release version. Thanks, --Patric > -Original Message- > From: Zhao, Patric [mailto:patric.z...@intel.com] > Sent: Wednesday, November 28, 2018 8:07 PM > To: dev@mxnet.incubator.apache.org > Subject: LSTM regression (was RE: Include MKLDNN into default mxnet pip > package) > > Hi Anirudh, > > The LSTM performance bug is fixed by MKL-DNN and PR in here > (https://github.com/apache/incubator-mxnet/pull/13417). > > I am still working on MKL-DNN team to get a patch release for MXNet 1.4 in > 1 or 2 days. > > Will update the status soon. > > Thanks everyone. > > --Patric > > > -Original Message- > > From: Anirudh Subramanian [mailto:anirudh2...@gmail.com] > > Sent: Tuesday, November 27, 2018 6:16 AM > > To: dev@mxnet.incubator.apache.org > > Subject: Re: Include MKLDNN into default mxnet pip package > > > > Hi Tao, > > > > I agree with Steffen that we can start with a stable release for > > MKLDNN for 1.4.0. For your suggestion on using 0.17, can you provide > > info on what versioning mechanism MKLDNN uses. Once a MKLDNN > release > > is out and there are some regressions found like the LSTM regression, > > would it be possible to do a patch release for it or maintain a release > branch for it ? > > > > Anirudh > > > > On Sun, Nov 25, 2018 at 5:03 PM Lv, Tao A wrote: > > > > > Hi Steffen, > > > > > > I think all the commits on MKL-DNN master branch are well tested for > > > MKL-DNN development team. If we really want to have a release commit > > > in the coming 1.4 mxnet release, my suggestion is 0.17 MKL-DNN release. > > > > > > Thank you, > > > Tao > > > > > > Sent from my iPhone > > > > > > > On Nov 26, 2018, at 8:09 AM, Steffen Rochel > > > > > > > wrote: > > > > > > > > +1 to make MKL-DNN default. > > > > I'm tracking > > > > https://github.com/apache/incubator-mxnet/issues/13369 > > > > as open issue to be addressed for 1.4.0 I do agree that we should > > > > move to a model to include released > > > dependencies > > > > instead of just taking bleeding edge snapshots. > > > > However, speed of development is important as well. > > > > As a compromise for 1.4.0 release with MKL-DNN: can the MKL-DNN > > > development > > > > team provide us with a well tested tag/commit id to include in > > > > 1.4.0 release? > > > > Steffen > > > > > > > >> On Wed, Nov 21, 2018 at 11:42 PM Lv, Tao A > > wrote: > > > >> > > > >> Thanks for the information, Kellen and Naveen. > > > >> > > > >> Better than onnx-tensorrt, MKL-DNN has already provided > > > >> versioning and release tags. My concern is that as MKL-DNN is > > > >> still under intensive development, if it has a new feature or bug > > > >> fix on its master branch, > > > do we > > > >> really want to wait for next release to get it supported in MXNet? > > > >> > > > >> Take the LSTM regression as an example, probably MKL-DNN will > > > >> give a fix or improvement on its master branch soon, do we need > > > >> to wait for 0.18 release to get it fixed for mxnet user? AFAIK, > > > >> tensorflow is also using normal commit id, not release, as the > > > >> dependency for MKL- > > DNN. > > > >> > > > >> Regarding the LSTM regression, we are using internal JIRA tickets > > > >> rather than github issues to track the defects of MKL-DNN. But I > > > >> agree with > > > you, > > > >> we need update the progress of it in Alex's issue. > > > >> > > > >> Thanks, > > > >> -tao > > > >> > > > >> -Original Message- &g
RE: LSTM regression (was RE: Include MKLDNN into default mxnet pip package)
MKL-DNN v0.17.1 is released https://github.com/intel/mkl-dnn/tree/v0.17.1 I have submitted the PR to pin this release version. Thanks, --Patric > -Original Message- > From: Zhao, Patric [mailto:patric.z...@intel.com] > Sent: Wednesday, November 28, 2018 8:07 PM > To: dev@mxnet.incubator.apache.org > Subject: LSTM regression (was RE: Include MKLDNN into default mxnet pip > package) > > Hi Anirudh, > > The LSTM performance bug is fixed by MKL-DNN and PR in here > (https://github.com/apache/incubator-mxnet/pull/13417). > > I am still working on MKL-DNN team to get a patch release for MXNet 1.4 in > 1 or 2 days. > > Will update the status soon. > > Thanks everyone. > > --Patric > > > -Original Message- > > From: Anirudh Subramanian [mailto:anirudh2...@gmail.com] > > Sent: Tuesday, November 27, 2018 6:16 AM > > To: dev@mxnet.incubator.apache.org > > Subject: Re: Include MKLDNN into default mxnet pip package > > > > Hi Tao, > > > > I agree with Steffen that we can start with a stable release for > > MKLDNN for 1.4.0. For your suggestion on using 0.17, can you provide > > info on what versioning mechanism MKLDNN uses. Once a MKLDNN > release > > is out and there are some regressions found like the LSTM regression, > > would it be possible to do a patch release for it or maintain a release > branch for it ? > > > > Anirudh > > > > On Sun, Nov 25, 2018 at 5:03 PM Lv, Tao A wrote: > > > > > Hi Steffen, > > > > > > I think all the commits on MKL-DNN master branch are well tested for > > > MKL-DNN development team. If we really want to have a release commit > > > in the coming 1.4 mxnet release, my suggestion is 0.17 MKL-DNN release. > > > > > > Thank you, > > > Tao > > > > > > Sent from my iPhone > > > > > > > On Nov 26, 2018, at 8:09 AM, Steffen Rochel > > > > > > > wrote: > > > > > > > > +1 to make MKL-DNN default. > > > > I'm tracking > > > > https://github.com/apache/incubator-mxnet/issues/13369 > > > > as open issue to be addressed for 1.4.0 I do agree that we should > > > > move to a model to include released > > > dependencies > > > > instead of just taking bleeding edge snapshots. > > > > However, speed of development is important as well. > > > > As a compromise for 1.4.0 release with MKL-DNN: can the MKL-DNN > > > development > > > > team provide us with a well tested tag/commit id to include in > > > > 1.4.0 release? > > > > Steffen > > > > > > > >> On Wed, Nov 21, 2018 at 11:42 PM Lv, Tao A > > wrote: > > > >> > > > >> Thanks for the information, Kellen and Naveen. > > > >> > > > >> Better than onnx-tensorrt, MKL-DNN has already provided > > > >> versioning and release tags. My concern is that as MKL-DNN is > > > >> still under intensive development, if it has a new feature or bug > > > >> fix on its master branch, > > > do we > > > >> really want to wait for next release to get it supported in MXNet? > > > >> > > > >> Take the LSTM regression as an example, probably MKL-DNN will > > > >> give a fix or improvement on its master branch soon, do we need > > > >> to wait for 0.18 release to get it fixed for mxnet user? AFAIK, > > > >> tensorflow is also using normal commit id, not release, as the > > > >> dependency for MKL- > > DNN. > > > >> > > > >> Regarding the LSTM regression, we are using internal JIRA tickets > > > >> rather than github issues to track the defects of MKL-DNN. But I > > > >> agree with > > > you, > > > >> we need update the progress of it in Alex's issue. > > > >> > > > >> Thanks, > > > >> -tao > > > >> > > > >> -Original Message- > > > >> From: kellen sunderland [mailto:kellen.sunderl...@gmail.com] > > > >> Sent: Thursday, November 22, 2018 10:55 AM > > > >> To: dev@mxnet.incubator.apache.org > > > >> Subject: Re: Include MKLDNN into default mxnet pip package > > > >> > > > >> Agree with your point about other repos also not being based on > > > versioning > > > >> Tao. I would point out that I've given some that I've worked > > > &
LSTM regression (was RE: Include MKLDNN into default mxnet pip package)
Hi Anirudh, The LSTM performance bug is fixed by MKL-DNN and PR in here (https://github.com/apache/incubator-mxnet/pull/13417). I am still working on MKL-DNN team to get a patch release for MXNet 1.4 in 1 or 2 days. Will update the status soon. Thanks everyone. --Patric > -Original Message- > From: Anirudh Subramanian [mailto:anirudh2...@gmail.com] > Sent: Tuesday, November 27, 2018 6:16 AM > To: dev@mxnet.incubator.apache.org > Subject: Re: Include MKLDNN into default mxnet pip package > > Hi Tao, > > I agree with Steffen that we can start with a stable release for MKLDNN for > 1.4.0. For your suggestion on using 0.17, can you provide info on what > versioning mechanism MKLDNN uses. Once a MKLDNN release is out and > there are some regressions found like the LSTM regression, would it be > possible to do a patch release for it or maintain a release branch for it ? > > Anirudh > > On Sun, Nov 25, 2018 at 5:03 PM Lv, Tao A wrote: > > > Hi Steffen, > > > > I think all the commits on MKL-DNN master branch are well tested for > > MKL-DNN development team. If we really want to have a release commit > > in the coming 1.4 mxnet release, my suggestion is 0.17 MKL-DNN release. > > > > Thank you, > > Tao > > > > Sent from my iPhone > > > > > On Nov 26, 2018, at 8:09 AM, Steffen Rochel > > > > > wrote: > > > > > > +1 to make MKL-DNN default. > > > I'm tracking https://github.com/apache/incubator-mxnet/issues/13369 > > > as open issue to be addressed for 1.4.0 I do agree that we should > > > move to a model to include released > > dependencies > > > instead of just taking bleeding edge snapshots. > > > However, speed of development is important as well. > > > As a compromise for 1.4.0 release with MKL-DNN: can the MKL-DNN > > development > > > team provide us with a well tested tag/commit id to include in 1.4.0 > > > release? > > > Steffen > > > > > >> On Wed, Nov 21, 2018 at 11:42 PM Lv, Tao A > wrote: > > >> > > >> Thanks for the information, Kellen and Naveen. > > >> > > >> Better than onnx-tensorrt, MKL-DNN has already provided versioning > > >> and release tags. My concern is that as MKL-DNN is still under > > >> intensive development, if it has a new feature or bug fix on its > > >> master branch, > > do we > > >> really want to wait for next release to get it supported in MXNet? > > >> > > >> Take the LSTM regression as an example, probably MKL-DNN will give > > >> a fix or improvement on its master branch soon, do we need to wait > > >> for 0.18 release to get it fixed for mxnet user? AFAIK, tensorflow > > >> is also using normal commit id, not release, as the dependency for MKL- > DNN. > > >> > > >> Regarding the LSTM regression, we are using internal JIRA tickets > > >> rather than github issues to track the defects of MKL-DNN. But I > > >> agree with > > you, > > >> we need update the progress of it in Alex's issue. > > >> > > >> Thanks, > > >> -tao > > >> > > >> -Original Message- > > >> From: kellen sunderland [mailto:kellen.sunderl...@gmail.com] > > >> Sent: Thursday, November 22, 2018 10:55 AM > > >> To: dev@mxnet.incubator.apache.org > > >> Subject: Re: Include MKLDNN into default mxnet pip package > > >> > > >> Agree with your point about other repos also not being based on > > versioning > > >> Tao. I would point out that I've given some that I've worked with > > similar > > >> feedback: https://github.com/onnx/onnx-tensorrt/issues/68 > > >> > > >>> On Wed, Nov 21, 2018 at 6:48 PM Naveen Swamy > > > wrote: > > >>> > > >>> Tao, > > >>> > > >>> You are right there are many submodules in 3rd party. We have to > > >>> start somewhere and I believe this one is a good candidate to start > with. > > >>> This is not to cater to release of MXNet or to tie them with the > > >>> releases of the submodules but instead to pick only stable > > >>> releases and not to pick up bleeding edge commits from the tip of > > >>> the master, this gives us confidence in the submodule that MXNet > > >>> users are depending on that especially if we make MKLDNN the default. > > >>> > > >>> Good to know it is known already as a regression.Alex has created > > >>> this issue https://github.com/apache/incubator-mxnet/issues/13369, > > >>> please add details and link the corresponding issue in MKLDNN(I > > >>> couldn't > > find). > > >>> > > >>> -Naveen > > >>> > > On Wed, Nov 21, 2018 at 6:04 PM Lv, Tao A > wrote: > > > > Here are my answers for the questions from Kellen and Naveen > > about MKL-DNN. It doesn't mean that I'm supportive for making > > MKL-DNN default here. > > > > @Kellen, > > > > FYI, here is a list for those platforms which are officially > > supported by MKL-DNN. > > https://github.com/intel/mkl-dnn#system-requirements > > > > Most of computation intensive kernels in MKL-DNN are JITed. So > > they are supposed to generate code according to the platform > > during runtime. For non-JIT code in MKL-DNN, sa
