mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mu Li <muli....@gmail.com>
Subject Re: AWS contributing ONNX-MXNet
Date Fri, 17 Nov 2017 03:49:46 GMT
Thanks for pointing it out, Hagay. I actually missed the "Special thanks to
dmlc/nnvm team" sentence. I was looking for Zhi and Tianqi's names, because
the onnx converter was mainly done by both of them. However I do not
entirely agree that I "had no comments". I actually pointed out the initial
draft is similar to the nnvm tutorial two days ago. Hagay made a few
updates, but unfortunately, I didn't have a chance to have a look at the
updated version before the announcement. Otherwise, I should leave a
comment to put individual contributors names on it.

On Thu, Nov 16, 2017 at 7:03 PM, Lupesko, Hagay <lupesko@gmail.com> wrote:

> Chiming in as well.
>
> First and foremost, I agree wholeheartedly that acknowledgments are due
> when deserved. In fact, we took care to add acknowledgments in the code,
> and in the blog post for that precise reason!
> I also personally talked with Mu, to make sure these are in order and
> appropriate, and he had no comments.
> Have we missed acknowledgments? Maybe (more on that below). But why assume
> this was done on intention?
>
> Addressing specific points (I won’t repeat Henri’s points):
> - I’m happy to take another look and see whether more files need to have
> the “ack” statement. But looking into it again, import_onnx.py [1] is the
> only one that seem to have been missed, and ack was already added. Sheng –
> I’ll grab some time with you Monday to discuss in details.
> - The tutorial itself was actually referenced from PyTorch, not nnvm. This
> is acknowledged by onnx-mxnet code, as well as the nnvm code.
> - We intentionally ack-ed an open source community (dmlc/nnvm) and not
> individuals. There’s more than Tianqi and Zhi that worked on nnvm and onnx,
> it is a whole community that we thank to.
> - “I was wondering why your below email didn't include such
> acknowledgement?” – as noted by Hen, the email did include the ack.
>
> One last thing, quoting Sheng: “In general, to have a healthy community, I
> believe the right things to do would be…”
> I would stress out that in order to have a healthy community, we should
> always assume others have best intentions – this will make us a stronger
> community, one that works together, and one that if fun to be part of.
>
> Hagay
>
> [1] https://github.com/onnx/onnx-mxnet/blob/master/onnx_mxnet/
> import_onnx.py
>
> On 11/16/17, 18:06, "Hen" <bayard@apache.org> wrote:
>
>     On Thu, Nov 16, 2017 at 4:32 PM, Sheng Zha <szha.pvg@gmail.com> wrote:
>
>     > Hi Hagay,
>     >
>     > (cc'd Zhi, Tianqi to make sure real authors are aware)
>     >
>     >
>     >
>     > At first glance the code in the repo you shared (i.e.
>     > https://github.com/onnx/onnx-mxnet) looks very
>     >
>     > familiar, so I did some searching. It looks like *almost all* the
> code
>     > are adopted from the *nnvm onnx*
>     >
>     > frontend, but the main contributor (*Zhi Zhang*, committer of mxnet,
> and
>     > intern at AWS) from this same
>     >
>     > community was not given his due credit in your email. To elaborate
> on why
>     > I think almost all the
>     >
>     > onnx-mxnet code is from nnvm onnx frontend:
>     >
>     >
>     >
>     > The following is the content of this repo:
>     >
>     > ├── LICENSE.txt
>     >
>     > ├── README.md
>     >
>     > ├── onnx_mxnet
>     >
>     > │   ├── __init__.py
>     >
>     > │   ├── common.py
>     >
>     > │   ├── import_helper.py
>     >
>     > │   ├── import_onnx.py
>     >
>     > │   └── tests
>     >
>     > │       ├── test_models.py
>     >
>     > │       └── test_super_resolution.py
>     >
>     > ├── setup.py
>     >
>     > ├── super_res_input.jpg
>     >
>     > └── super_res_output.jpg
>     >
>     > (Also attached a screenshot of the commit history of onnx_mxnet at
> the
>     > moment, as well as a copy of the git package, in case commit hash
> mismatch
>     > happens)
>     >
>     >
>     >
>     >    - Out of the 6 files under onnx_mxnet package
>     >       - the following two files are marked as being derived from
> nnvm:
>     >          - common.py
>     >          <https://github.com/onnx/onnx-mxnet/blob/master/onnx_mxnet/
> common.py#L12>
>     >          - import_helper.py
>     >          <https://github.com/onnx/onnx-mxnet/blob/master/onnx_mxnet/
> import_helper.py#L12>
>     >       - the rest four files that are not marked as being derived from
>     >       nnvm:
>     >          - __init__.py
>     >          <https://github.com/onnx/onnx-mxnet/blob/
> 4ebc02e8f1cc523049f0928b6dbc566a93dd2f47/onnx_mxnet/__init__.py#L15>:
>     >          looks like a copy from nnvm/frontend/onnx.py
>     >          <https://github.com/dmlc/nnvm/blob/
> 3da53e46db57c438b05fbebe8aa332ee8c5994d1/python/nnvm/frontend/onnx.py#L392
> >
>     >          - import_onnx.py
>     >          <https://github.com/onnx/onnx-mxnet/blob/
> 4ebc02e8f1cc523049f0928b6dbc566a93dd2f47/onnx_mxnet/import_onnx.py#L18>
> looks
>     >          like a copy from nnvm/frontend/onnx.py
>     >          <https://github.com/dmlc/nnvm/blob/
> 3da53e46db57c438b05fbebe8aa332ee8c5994d1/python/nnvm/frontend/onnx.py#L345
> >
>     >          - tests/test_models.py
>     >          <https://github.com/onnx/onnx-mxnet/blob/
> 4ebc02e8f1cc523049f0928b6dbc566a93dd2f47/onnx_mxnet/tests/test_models.py>
>     >             - this part
>     >             <https://github.com/onnx/onnx-mxnet/blob/
> 4ebc02e8f1cc523049f0928b6dbc566a93dd2f47/onnx_mxnet/tests/
> test_models.py#L35> looks
>     >             like a copy from nnvm frontend/onnx/model_zoo/__
> init__.py
>     >             <https://github.com/dmlc/nnvm/blob/
> 3da53e46db57c438b05fbebe8aa332ee8c5994d1/tests/python/
> frontend/onnx/model_zoo/__init__.py#L7>
>     >             - this part
>     >             <https://github.com/onnx/onnx-mxnet/blob/
> 4ebc02e8f1cc523049f0928b6dbc566a93dd2f47/onnx_mxnet/tests/
> test_models.py#L63> looks
>     >             like a copy from nnvm frontend/onnx/test_forward.py
>     >             <https://github.com/dmlc/nnvm/blob/
> 3da53e46db57c438b05fbebe8aa332ee8c5994d1/tests/python/
> frontend/onnx/test_forward.py#L9>
>     >          - tests/test_super_resolution.py
>     >          <https://github.com/onnx/onnx-mxnet/blob/
> 4ebc02e8f1cc523049f0928b6dbc566a93dd2f47/onnx_mxnet/tests/
> test_super_resolution.py#L24> looks
>     >          like a copy from nnvm tutorials/from_onnx.py
>     >          <https://github.com/dmlc/nnvm/blob/
> 3da53e46db57c438b05fbebe8aa332ee8c5994d1/tutorials/from_onnx.py#L22>
>     >
>     >
>     >
>     >
>     >
>     > Looks like in the blog post here
>     > <https://aws.amazon.com/blogs/ai/announcing-onnx-support-
> for-apache-mxnet/>,
>     > there is one line giving credit to nnvm authors:
>     >
>     > *"Special thanks to the dmlc/nnvm community, whose ONNX code was
> used as a
>     > reference for this implementation."*
>     >
>     >
>     >
>     > I was wondering why your below email didn't include such
> acknowledgement?
>     > In general, to have a healthy community, I believe the right things
> to do
>     > would be:
>     >
>     > 1. Give the due credit to the original authors in communications.
>     >
>     > 2. Keep original authors' names in the file licenses, which is a
> required
>     > for apache 2.0 license <http://www.apache.org/licenses/LICENSE-2.0>
> that
>     > is used in both packages.
>     >
>     > 3. For code that you reorganized, keep a reference of the source from
>     > which it’s adopted, which is good practice when contributing
> open-source.
>     >
>
>     On #2 - the Apache license does not require that author's names be
> kept in
>     files (I presume you're referring to the Author comment in the tutorial
>     file as I didn't see any others).
>
>     The Apache license requires that (section 4):
>
>     - You must give any other recipients of the Work or Derivative Works a
> copy
>     of this License; and
>     - You must cause any modified files to carry prominent notices stating
> that
>     You changed the files; and
>     - You must retain, in the Source form of any Derivative Works that You
>     distribute, all copyright, patent, trademark, and attribution notices
> from
>     the Source form of the Work, excluding those notices that do not
> pertain to
>     any part of the Derivative Works; and
>     - If the Work includes a "NOTICE" text file as part of its
> distribution,
>     then any Derivative Works that You distribute must include a readable
> copy
>     of the attribution notices contained within such NOTICE file,
>
>     Going through each in turn:
>
>     1) The repo contains a copy of the Apache 2.0 license,
>     2) 2 of the 6 files contain prominent notices; I agree that the other 4
>     should contain a prominent notice if they're derived (I haven't
> compared
>     the files, but I don't doubt your research),
>     3) There are no additional copyright, patent, trademark or attribution
>     notices in the files copied (this is why Apache requires a source
> header
>     for its projects; DMLC does not appear to have source headers on their
>     files, its equivalent of Apache's legal committee should strongly think
>     about that standardizing on that for all its projects imo);
>     4) There is no NOTICE file (again, imo the DMLC legal thinkers should
>     strongly think about having NOTICE files imo).
>
>     So agreed that there's a change to make, but there's no requirement to
> keep
>     author's names in files. Note that there is a practice (and it's a
>     recommended or even mandatory legal practice, I'm not sure which) to
> keep a
>     Copyright Owner's name in a file, but author and copyright owner are
>     different concepts.
>
>     Hen
>
>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message