Let's cancel this vote for now and move the OMP conversation to a discuss thread as suggested here in order not to antagonize anyone. Let's find the best solution, this is not about one upping anyone but to gather facts and document what's going on.
Chris, would you be so kind to reopen the PR so we can keep gathering facts to understand and reproduce your results so we can have a constructive discussion? I think this would be a compromise to move forward in a non confrontational way. Thank you. Pedro. On Tue, Jun 18, 2019 at 1:35 PM Pedro Larroy <pedro.larroy.li...@gmail.com> wrote: > > Good suggestion. I thought the issue was discussed at length but I > agree might be better to softly bring them to the list as a discuss > first instead of a direct vote. Will do so in the future. > > Thanks. > > > On Tue, Jun 18, 2019 at 12:56 PM Tianqi Chen <tqc...@cs.washington.edu> wrote: > > > > In a situation like this, I would suggest a DISCUSS thread is more proper > > before the voting one. > > As the tension can generally be high in a voting thread and it is hard to > > make a technical decision without previous discussions and pros/cons being > > listed. > > > > Tianqi > > > > On Fri, Jun 14, 2019 at 4:59 PM Pedro Larroy <pedro.larroy.li...@gmail.com> > > wrote: > > > > > Hi all > > > > > > This is a 5-day vote to act and wrap up an outstanding PR that removes > > > linkage with multiple OpenMP from 3rdparty and uses the system > > > provided one which might resolve a number of difficult to debug issues > > > and possible undefined behaviour. > > > > > > https://github.com/apache/incubator-mxnet/pull/12160 > > > > > > See the comments in the thread for more details but in short, linking > > > with multiple openmp versions seems to lead to undefined behaviour, > > > plus it would simplify not having to deal with a custom openmp version > > > and rely on the platform provided one. > > > > > > This is expected to simplify builds and address a number of problems. > > > Seems it doesn't cause any performance degradation, (the Gluon tests > > > run almost 4x faster in my 64 core machine). > > > > > > There has been in depth study of performance implications by > > > contributors like Stanislav Tsukrov and Anton Chernov. All the > > > concerns and comments from the reviewers have been addressed and we > > > can't keep asking open ended questions to block PRs. Reviewers are > > > expected to be proactive and responsive to contributors so we keep > > > encouraging active contributors. > > > > > > please vote to merge this PR accordingly: > > > > > > +1 = approve > > > +0 = no opinion > > > -1 = disapprove (provide reason) > > > > > > If we observe regressions reported by any internal performance systems > > > or by contributors the PR can be reverted easily. So it's not a one > > > way door. But it will be useful to try this in master for a while. > > > > > > Thank you. > > > > > > Pedro. > > >
