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 > > > > > > > > > > > > > > > > > > > > > > > > > > > >
