Hi Marco,

The backend private APIs in engine, executor, storage, ndarray etc. can
still be changed.
I understand that it may introduce code duplication, but introducing
duplicate C APIs can still be better than the backend
developer having to worry about different frontends. Not to mention a
frontend which is not yet merged to the
repo but in its own repo, these repos should also be considered consumers
of MXNet API.

Anirudh

On Thu, Apr 11, 2019 at 12:12 PM Marco de Abreu <marco.g.ab...@gmail.com>
wrote:

> Good point about the adoption speed for the different frontends, Anirudh.
> While this is a quite valid argument, I'm afraid of the complexity it might
> introduce as well as a risk of further diverging frontend functionality.
>
> I'd rather propose that we introduce a guideline to follow when changes to
> C-APIs are being made. Part of that could be starting a thread like this
> one that lays down the changes that are being made to the C-API. We could
> then coordinate the changes to the different frontends and gather people
> from the community who feel comfortable to do the changes in the respective
> frontends. If nobody speaks up, the original proposer of that change could
> be responsible to do the necessary changes.
>
> An adjacent topic for this discussion could be test coverage: We currently
> have no tools to determine which frontend hits which C-API and where
> changes have to be made. This might be a topic we should spark up again
> separately.
>
> -Marco
>
> On Thu, Apr 11, 2019 at 8:55 PM Marco de Abreu <marco.g.ab...@gmail.com>
> wrote:
>
> > My personal opinion towards that discussion is that we should keep the
> > C-API free from semantic versioning because otherwise we're introducing
> two
> > "fronts" that we have to maintain backwards compatibility for. By the
> way,
> > currently, we have no way to verify and guarantee the compatibility of
> the
> > C-API. The major issue I'd see with adding SemVer for the C-API is that
> > this would increase the complexity of changes that are (in my opinion)
> > entirely internal to MXNet by introducing another thing that developers
> > would have to look out for - possibly introducing code duplication as
> > described by Jun while not providing any clear benefits to me.
> >
> > If there is a use-case where people can not even use our C++ package,
> then
> > we could have discussions about introducing a user-facing C-API, but
> right
> > now this approach to interface with our C-API (although I know that
> people
> > use it) seem a bit like using undocumented Windows APIs: They work, but
> > it's on your own risk, they might always break and there's no guarantee.
> >
> > -Marco
> >
> > On Thu, Apr 11, 2019 at 8:52 PM Anirudh Subramanian <
> anirudh2...@gmail.com>
> > wrote:
> >
> >> Hi Jun,
> >>
> >> Till now from what I have observed this has been an undocumented
> guideline
> >> to not break C APIs (example:
> >>
> https://github.com/apache/incubator-mxnet/pull/11429#discussion_r199564999
> >> ).
> >> Although the C APIs are supposed to serve only as bridges for frontend
> >> language bindings (exception being C Predict API), I think there are 3rd
> >> party libraries like Horovod which are starting to
> >> depend on it (https://github.com/apache/incubator-mxnet/pull/14615) .
> >>
> >> Also, since MXNet has a lot of frontend bindings ensuring backward
> >> compatibility with semver can help frontend bindings adopt the new APIs
> at
> >> their own pace.
> >>
> >> Anirudh
> >>
> >>
> >> On Thu, Apr 11, 2019 at 10:58 AM Jun Wu <wujun....@gmail.com> wrote:
> >>
> >> > I'm not sure about whether C APIs should fall under semver. This is
> the
> >> > discussion we would like to have with the community.
> >> >
> >> > My thinking on this:
> >> > 1. In most of the cases, C APIs only serve as bridges between frontend
> >> > language bindings and C++ backend. Most of users/developers do not
> >> interact
> >> > directly with C APIs.
> >> > 2. The cases I can think of where C APIs are directly adopted in
> >> > application development are model deployment in a C/C++ environment.
> In
> >> > those cases, developers only interact with C Predict APIs, which we
> >> didn't
> >> > touch.
> >> >
> >> > If the community feel that we are obliged to keep the semver for all C
> >> > APIs, we can try to make a copy of the C APIs we intend to modify in
> >> the PR
> >> > and keep the old signatures intact, this will introduce a lot of
> >> duplicate
> >> > code though.
> >> >
> >> > On Thu, Apr 11, 2019 at 8:50 AM Anirudh Subramanian <
> >> anirudh2...@gmail.com
> >> > >
> >> > wrote:
> >> >
> >> > > I was under the impression that C API does fall under semver. Has
> this
> >> > been
> >> > > discussed somewhere before ? Is this also the case for C Predict
> API ?
> >> > >
> >> > > On Thu, Apr 11, 2019, 8:08 AM Marco de Abreu <
> marco.g.ab...@gmail.com
> >> >
> >> > > wrote:
> >> > >
> >> > > > In case only changes to the c-api are being made, it doesn't fall
> >> under
> >> > > our
> >> > > > semantic versioning since that's not a user facing API and thus
> I'd
> >> be
> >> > in
> >> > > > favour as doing it as part of a minor release. If there is any
> >> > > behavioural
> >> > > > change from a user perspective (a good indicator would be if tests
> >> have
> >> > > to
> >> > > > be changed as reaction to the Backend changes), then I'd prefer a
> >> major
> >> > > > release.
> >> > > >
> >> > > > I'd slightly prefer a minor release since this change touches
> quite
> >> a
> >> > few
> >> > > > parts and could risk being outdated/diverged as the time until 2.0
> >> > > > progresses.
> >> > > >
> >> > > > -Marco
> >> > > >
> >> > > > Aaron Markham <aaron.s.mark...@gmail.com> schrieb am Do., 11.
> Apr.
> >> > 2019,
> >> > > > 16:28:
> >> > > >
> >> > > > > Just curious about when this kind of change will land. Would it
> >> wait
> >> > > for
> >> > > > > 2.0 or would it be in 1.5 or another minor release?
> >> > > > >
> >> > > > > On Thu, Apr 11, 2019, 00:15 Junru Shao <junrushao1...@gmail.com
> >
> >> > > wrote:
> >> > > > >
> >> > > > > > Really nice improvement over MXNet's usability! I suggest that
> >> we
> >> > > could
> >> > > > > > make numpy-compatible behavior default in 2.0.
> >> > > > > >
> >> > > > > > On Wed, Apr 10, 2019 at 11:34 PM Jun Wu <wujun....@gmail.com>
> >> > wrote:
> >> > > > > >
> >> > > > > > > Dear Community,
> >> > > > > > >
> >> > > > > > > A while ago, we sent out an RFC
> >> > > > > > > <https://github.com/apache/incubator-mxnet/issues/14253>
> >> > > discussing
> >> > > > > the
> >> > > > > > > initiative introducing NumPy compatibility into MXNet. As
> the
> >> > first
> >> > > > > > outcome
> >> > > > > > > of this initiative, we submitted the PR
> >> > > > > > > <https://github.com/apache/incubator-mxnet/pull/14661>
> >> providing
> >> > > the
> >> > > > > > > infrastructure of supporting zero-dim (scalar) and zero-size
> >> > > tensors,
> >> > > > > > which
> >> > > > > > > have been long-missing in MXNet.
> >> > > > > > >
> >> > > > > > > In our implementation, we have put the best efforts of
> keeping
> >> > the
> >> > > > > > promise
> >> > > > > > > of backward compatibility in all the language bindings.
> >> > > Nevertheless,
> >> > > > > we
> >> > > > > > > still would like to call out the changes explicitly that may
> >> > impact
> >> > > > > your
> >> > > > > > > existing codebases developed on top of MXNet by calling
> C-APIs
> >> > > > directly
> >> > > > > > or
> >> > > > > > > implementing operators in your own repos.
> >> > > > > > >
> >> > > > > > > 1. In you application, if you called any one of the
> following
> >> > > > > > shape-related
> >> > > > > > > C-APIs, you will need to change the data type of shape's
> ndim
> >> and
> >> > > > > > dim_size
> >> > > > > > > from *unsigned int* to signed *int*, because we have to use
> >> -1 to
> >> > > > > > represent
> >> > > > > > > unknown shape information, and reserve 0 for scalar and
> >> zero-size
> >> > > > > > tensors.
> >> > > > > > > One example of such changes can be seen in the cpp-package
> >> > > > > > > <
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/incubator-mxnet/pull/14661/files#diff-c0e77771fcfe1619faa4ff5f59d94e8bR183
> >> > > > > > > >
> >> > > > > > > calling MXSymbolInferShape.
> >> > > > > > > - MXSymbolInfershape
> >> > > > > > > - MXSymbolInfershapePartial
> >> > > > > > > - MXExecutorSimpleBind
> >> > > > > > > - MXExecutorReshape
> >> > > > > > > - MXNDArrayGetShape
> >> > > > > > > - MXNDArrayCreaetFromSharedMem
> >> > > > > > >
> >> > > > > > > 2. If you have implemented operators in your own codebases,
> >> you
> >> > > will
> >> > > > > > > probably need to change every operator's shape inference
> >> function
> >> > > to
> >> > > > > use
> >> > > > > > > the following util functions to check whether shape
> >> information
> >> > is
> >> > > > > known,
> >> > > > > > > instead of checking against 0 directly. One example of such
> >> > changes
> >> > > > can
> >> > > > > > be
> >> > > > > > > seen in the shape inference function
> >> > > > > > > <
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/incubator-mxnet/pull/14661/files#diff-afa640c4653c59f00f43a84455f91ef9R35
> >> > > > > > > >
> >> > > > > > > of concat operator.
> >> > > > > > > - shape_is_known (include/mxnet/tuple.h)
> >> > > > > > > - ndim_is_known (include/mxnet/tuple.h)
> >> > > > > > > - dim_size_is_known (include/mxnet/tuple.h)
> >> > > > > > >
> >> > > > > > > If you are interested in knowing the value of scalar
> tensors,
> >> and
> >> > > > hence
> >> > > > > > > understanding our motivation further, this thread
> >> > > > > > > <
> >> > > >
> >> https://discuss.mxnet.io/t/rank-0-arrays-in-mxnet-aka-pi-is-wrong/108
> >> > > > > >
> >> > > > > > of
> >> > > > > > > discussion provides very good insights from the view of data
> >> > > science.
> >> > > > > It
> >> > > > > > > was actually related to an opportunity for MXNet becoming
> the
> >> > > backend
> >> > > > > of
> >> > > > > > > PyMC <https://en.wikipedia.org/wiki/PyMC3>, but somehow it
> >> > didn't
> >> > > go
> >> > > > > > > through due to missing several key features
> >> > > > > > > <
> >> https://discuss.mxnet.io/t/moving-pymc3-from-theano-to-mxnet/86
> >> > >,
> >> > > > and
> >> > > > > > > scalar tensors is one of them.
> >> > > > > > >
> >> > > > > > > Please leave comments in the PR
> >> > > > > > > <https://github.com/apache/incubator-mxnet/pull/14661> if
> you
> >> > have
> >> > > > any
> >> > > > > > > concerns or suggestions of our work.
> >> > > > > > >
> >> > > > > > > Thank you very much for your time and consideration.
> >> > > > > > >
> >> > > > > > > Best,
> >> > > > > > > Jun
> >> > > > > > >
> >> > > > > > > *References*
> >> > > > > > > [1] RFC of NumPy compatibility:
> >> > > > > > > https://github.com/apache/incubator-mxnet/issues/14253
> >> > > > > > > [2] Pull request of supporting scalar and zero-size tensors:
> >> > > > > > > https://github.com/apache/incubator-mxnet/pull/14661
> >> > > > > > > [3] The value of scalar tensors from the view of data
> science:
> >> > > > > > >
> >> > > >
> >> https://discuss.mxnet.io/t/rank-0-arrays-in-mxnet-aka-pi-is-wrong/108
> >> > > > > > > [4] Previous discussion for MXNet becoming the backend of
> >> PyMC:
> >> > > > > > >
> >> https://discuss.mxnet.io/t/moving-pymc3-from-theano-to-mxnet/86
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>

Reply via email to