mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lupesko, Hagay" <lupe...@gmail.com>
Subject Re: AWS contributing ONNX-MXNet
Date Fri, 17 Nov 2017 04:04:10 GMT
Thanks for your notes Mu.

The tutorial on both nnvm and onnx-mxnet is referenced from ONNX PyTorch tutorial. And both
code repos ack that, as appropriate, and without including any specific names.
When I looked at the nnvm onnx code, the commit history shows more than just Tianqi and Zhi,
which made it seem more appropriate to refer to the entire dmlc/nnvm community.

Hagay 

On 11/16/17, 19:49, "Mu Li" <limu.cn@gmail.com on behalf of muli.cmu@gmail.com> wrote:

    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
View raw message