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 03:03:31 GMT
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