RE: Hold on the merge of new MKL-DNN operator tests

[Lv, Tao A]

Thanks for bringing this up, Steffen.

I tried to have a separate PR to mitigate this issue although I'm not a Windows 
expert. So please help to review and feel free to comment.

https://github.com/apache/incubator-mxnet/pull/13109

-tao

-Original Message-
From: Steffen Rochel [mailto:steffenroc...@gmail.com] 
Sent: Sunday, November 4, 2018 9:48 AM
To: dev@mxnet.incubator.apache.org
Subject: Re: Hold on the merge of new MKL-DNN operator tests

Thanks Tao. Please check also the compiler warnings, e.g.
http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/incubator-mxnet/branches/master/runs/1861/nodes/118/steps/843/log/?start=0
(Build GPU MKLDNN windows). We might just be missing the right compiler switch, 
e.g.

warning C4530: C++ exception handler used, but unwind semantics are not 
enabled. Specify /EHsc

Regards,

Steffen


On Sat, Nov 3, 2018 at 3:40 PM kellen sunderland < kellen.sunderl...@gmail.com> 
wrote:

> Hey Tao, thanks for letting the community know.  It's completely 
> understandable if you want to dig deep on the failure.  Don't worry 
> about taking a little extra time to get to the bottom of test 
> failures, that's exactly the reason we have the CI setup.  Let us know 
> if there's anything you think we can help with.
>
> On Fri, Nov 2, 2018 at 7:56 PM Lv, Tao A  wrote:
>
> >
> > Hi MXNet developers,
> >
> > I am working on PR#12953<
> > https://github.com/apache/incubator-mxnet/pull/12953> to update the 
> > version of MKL-DNN dependency. This PR will help to address several 
> > requests and issues from the commnunity of both MXNet and MKL-DNN. 
> > It
> will
> > also improve MXNet performance a lot when using MKL-DNN backend.
> Currently,
> > this work is almost done except the failure of LRN CPP test. Since 
> > this
> PR
> > doesn't touch the integration code of MKL-DNN LRN operator, so I 
> > guess there is something new in MKL-DNN which has conflict with the CPP 
> > test .
> An
> > internal discussion is ongoing with MKL-DNN developers about that 
> > and hopefully this issue will be fixed soon.
> >
> > I noticed that there are several other CPP tests for MKL-DNN 
> > operator are under review and they are following the some methodology of 
> > LRN CPP test.
> > To avoid more conflicts, I suggest to hold on the merge of these new
> tests
> > before the LRN issue in #12953 is clrealy addressed.
> >
> > Here is the list of these PRs:
> > https://github.com/apache/incubator-mxnet/pull/13084
> > https://github.com/apache/incubator-mxnet/pull/12985
> > https://github.com/apache/incubator-mxnet/pull/12884
> >
> > Let me know what do you think. Thanks.
> > -tao
> >
>

Re: Hold on the merge of new MKL-DNN operator tests

[Steffen Rochel]

Thanks Tao. Please check also the compiler warnings, e.g.
http://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/incubator-mxnet/branches/master/runs/1861/nodes/118/steps/843/log/?start=0
(Build GPU MKLDNN windows). We might just be missing the right compiler
switch, e.g.

warning C4530: C++ exception handler used, but unwind semantics are
not enabled. Specify /EHsc

Regards,

Steffen


On Sat, Nov 3, 2018 at 3:40 PM kellen sunderland <
kellen.sunderl...@gmail.com> wrote:

> Hey Tao, thanks for letting the community know.  It's completely
> understandable if you want to dig deep on the failure.  Don't worry about
> taking a little extra time to get to the bottom of test failures, that's
> exactly the reason we have the CI setup.  Let us know if there's anything
> you think we can help with.
>
> On Fri, Nov 2, 2018 at 7:56 PM Lv, Tao A  wrote:
>
> >
> > Hi MXNet developers,
> >
> > I am working on PR#12953<
> > https://github.com/apache/incubator-mxnet/pull/12953> to update the
> > version of MKL-DNN dependency. This PR will help to address several
> > requests and issues from the commnunity of both MXNet and MKL-DNN. It
> will
> > also improve MXNet performance a lot when using MKL-DNN backend.
> Currently,
> > this work is almost done except the failure of LRN CPP test. Since this
> PR
> > doesn't touch the integration code of MKL-DNN LRN operator, so I guess
> > there is something new in MKL-DNN which has conflict with the CPP test .
> An
> > internal discussion is ongoing with MKL-DNN developers about that and
> > hopefully this issue will be fixed soon.
> >
> > I noticed that there are several other CPP tests for MKL-DNN operator are
> > under review and they are following the some methodology of LRN CPP test.
> > To avoid more conflicts, I suggest to hold on the merge of these new
> tests
> > before the LRN issue in #12953 is clrealy addressed.
> >
> > Here is the list of these PRs:
> > https://github.com/apache/incubator-mxnet/pull/13084
> > https://github.com/apache/incubator-mxnet/pull/12985
> > https://github.com/apache/incubator-mxnet/pull/12884
> >
> > Let me know what do you think. Thanks.
> > -tao
> >
>

Re: Hold on the merge of new MKL-DNN operator tests

[kellen sunderland]

Hey Tao, thanks for letting the community know.  It's completely
understandable if you want to dig deep on the failure.  Don't worry about
taking a little extra time to get to the bottom of test failures, that's
exactly the reason we have the CI setup.  Let us know if there's anything
you think we can help with.

On Fri, Nov 2, 2018 at 7:56 PM Lv, Tao A  wrote:

>
> Hi MXNet developers,
>
> I am working on PR#12953<
> https://github.com/apache/incubator-mxnet/pull/12953> to update the
> version of MKL-DNN dependency. This PR will help to address several
> requests and issues from the commnunity of both MXNet and MKL-DNN. It will
> also improve MXNet performance a lot when using MKL-DNN backend. Currently,
> this work is almost done except the failure of LRN CPP test. Since this PR
> doesn't touch the integration code of MKL-DNN LRN operator, so I guess
> there is something new in MKL-DNN which has conflict with the CPP test . An
> internal discussion is ongoing with MKL-DNN developers about that and
> hopefully this issue will be fixed soon.
>
> I noticed that there are several other CPP tests for MKL-DNN operator are
> under review and they are following the some methodology of LRN CPP test.
> To avoid more conflicts, I suggest to hold on the merge of these new tests
> before the LRN issue in #12953 is clrealy addressed.
>
> Here is the list of these PRs:
> https://github.com/apache/incubator-mxnet/pull/13084
> https://github.com/apache/incubator-mxnet/pull/12985
> https://github.com/apache/incubator-mxnet/pull/12884
>
> Let me know what do you think. Thanks.
> -tao
>

Hold on the merge of new MKL-DNN operator tests

[Lv, Tao A]

Hi MXNet developers,

I am working on PR#12953 
to update the version of MKL-DNN dependency. This PR will help to address 
several requests and issues from the commnunity of both MXNet and MKL-DNN. It 
will also improve MXNet performance a lot when using MKL-DNN backend. 
Currently, this work is almost done except the failure of LRN CPP test. Since 
this PR doesn't touch the integration code of MKL-DNN LRN operator, so I guess 
there is something new in MKL-DNN which has conflict with the CPP test . An 
internal discussion is ongoing with MKL-DNN developers about that and hopefully 
this issue will be fixed soon.

I noticed that there are several other CPP tests for MKL-DNN operator are under 
review and they are following the some methodology of LRN CPP test. To avoid 
more conflicts, I suggest to hold on the merge of these new tests before the 
LRN issue in #12953 is clrealy addressed.

Here is the list of these PRs:
https://github.com/apache/incubator-mxnet/pull/13084
https://github.com/apache/incubator-mxnet/pull/12985
https://github.com/apache/incubator-mxnet/pull/12884

Let me know what do you think. Thanks.
-tao