Hi, I have opened a PR for adding new content to 1.2.1 release notes: https://github.com/apache/incubator-mxnet/pull/11478 Please help review. Once this PR is approved I will be cutting the release.
Thanks, Anirudh On Tue, Jun 26, 2018 at 7:10 PM, Chris Olivier <cjolivie...@gmail.com> wrote: > +1 > > On Tue, Jun 26, 2018 at 5:50 PM Anirudh <anirudh2...@gmail.com> wrote: > > > Hi all, > > > > The current warning message for save_params: "save_params is deprecated, > > use save_parameters instead" is misleading > > for users who may use the API to load into SymbolBlock. To make it > clearer > > for all users, a better warning message is to include alternative to > > save_params and reference to detailed documentation. > > The message improvement is important, but not blocking to move forward to > > complete the voting process for MXNet v1.2.1 patch release. > > We plan to follow up with a 1.2.2 patch release to improve the message > and > > potentially include other bug fixes. > > Please let me know if you have any thoughts, questions or suggestions. > > > > > > Anirudh > > > > On Mon, Jun 25, 2018 at 10:44 PM, Anirudh <anirudh2...@gmail.com> wrote: > > > > > > > > Hi Mu, > > > > > > Thanks for bringing this up and hopefully this should answer Sheng's > > > question. > > > Thomas pointed out something similar in the PR here for the warning > > > message which I didn't notice back then: > > > https://github.com/apache/incubator-mxnet/pull/11127 > > > > > > Not sure about the reasoning to not add it and if there was an offline > > > discussion about this between Thomas and Erik. > > > It would be nice if you guys could pitch in if there were any strong > > > reasons. > > > > > > I understand that a more informed warning when using save_params would > > > really avoid some customer frustration. > > > Having said that, I am a little worried about the timeline though since > > > some customers are eagerly waiting for the release of 1.2.1. > > > Another RC would delay it by at-least one and a half weeks. > > > > > > Anirudh > > > > > > On Mon, Jun 25, 2018 at 9:54 PM, Mu Li <muli....@gmail.com> wrote: > > > > > >> Detailed documents should help, but the current warning message that > > >> "save_params is deprecated, use save_parameters instead" is not > > sufficient > > >> enough. > > >> > > >> Some details about the API changes: > > >> > > >> 1. v1.2 changed the implementation of "save_params", which is > explained > > in > > >> the release note. The main benefit for this change is that we don't > need > > >> to > > >> create layers within a name scope. [1] > > >> 2. we found this change breaks a gluon-to-symbol usage, even though we > > >> recommended users to use "export" for this usage. [2] > > >> 3. for some good reasons we made a decision to revert save_params in > > >> v1.2.1, and introduced a new API called save_parameters for this new > > >> behavior. [3] > > >> > > >> Since calling save_params each time will generate a warning message, > > it's > > >> a > > >> major API change. The recommended for users to update their codes are: > > >> > > >> 1. If you save parameters to load back into a SymbolBlock, you can use > > >> export instead, though keeping it will not break your codes except > for a > > >> warning message. (But it will break in v1.2) > > >> 2. If you create gluon layers without a name scope, you must replace > > >> save_params with save_parameters. Otherwise, your model cannot be > loaded > > >> back in v1.2.1 (though it works in v1.2) > > >> 3. For the rest case, such as models are created within a name scope, > > and > > >> the models are loaded into gluon (not symbolblock) later, recommend > > >> replacing save_params with save_parameteres. If you don't do it, > nothing > > >> will break in v1.2 and v1.2.1, but v1.2.1 will give you a warning > > message. > > >> > > >> This API changes in v1.2 and v1.2.1 are pretty tricky. Anirudh did a > > great > > >> job in capturing them in release notes. But I feel it's hard for users > > to > > >> understand the impacts. I suggest to improve the warning message to > "use > > >> export if you want to load into SymbolBlock, otherwise use > > >> save_parameters. > > >> For more details, refer to this URL". > > >> > > >> [1] https://github.com/apache/incubator-mxnet/releases/tag/1.2.0 > > >> [2] https://github.com/apache/incubator-mxnet/issues/11091 > > >> [3] https://github.com/apache/incubator-mxnet/pull/11127 > > >> > > >> On Mon, Jun 25, 2018 at 9:23 PM, Sheng Zha <szha....@gmail.com> > wrote: > > >> > > >> > Wouldn’t this break users who are on 1.2.0 and used our API > correctly? > > >> Why > > >> > do we have to revert load_params, given that it’s backward > compatible? > > >> > > > >> > -sz > > >> > > > >> > > On Jun 25, 2018, at 6:30 PM, Anirudh <anirudh2...@gmail.com> > wrote: > > >> > > > > >> > > Hi, > > >> > > > > >> > > 1.2.1 (load_params) is backward compatible with 1.1.0 not with > > 1.2.0. > > >> > > It does not adhere exactly with semver but it had to be made, to > > >> quickly > > >> > > help our customers who were using the APIs incorrectly. > > >> > > > > >> > > Anirudh > > >> > > > > >> > >> On Mon, Jun 25, 2018 at 5:42 PM, Sheng Zha <szha....@gmail.com> > > >> wrote: > > >> > >> > > >> > >> save_parameters didn't exist in 1.2.0 so its addition usually > isn't > > >> > >> supposed to happen in a patch release if we stick to semantic > > >> > versioning. I > > >> > >> couldn't find a discussion on this exception. Did it happen? > > >> > >> > > >> > >> Would people who used 1.2.0 to save models be able to load > > >> parameters in > > >> > >> 1.2.1 using the reverted load_params? (i.e. is it backward > > >> compatible) > > >> > >> > > >> > >> -sz > > >> > >> > > >> > >> > > >> > >>> On Mon, Jun 25, 2018 at 4:07 PM, Anirudh <anirudh2...@gmail.com > > > > >> > wrote: > > >> > >>> > > >> > >>> Hi Mu, > > >> > >>> > > >> > >>> The warining currently printed is "save_params is deprecated. > > Please > > >> > use > > >> > >>> save_parameters." > > >> > >>> Isn't this similar to what you are suggesting ? > > >> > >>> > > >> > >>> Anirudh > > >> > >>> > > >> > >>>> On Mon, Jun 25, 2018 at 3:47 PM, Mu Li <muli....@gmail.com> > > wrote: > > >> > >>>> > > >> > >>>> v1.2.1 will print a deprecating warning message when calling > > >> > >>>> save_params. We should tell users clearly to replace > > "save_params" > > >> > with > > >> > >>>> "save_parameters" or something else. > > >> > >>>> > > >> > >>>> On Mon, Jun 18, 2018 at 6:52 PM, Anirudh < > anirudh2...@gmail.com> > > >> > >> wrote: > > >> > >>>> > > >> > >>>>> Hi, > > >> > >>>>> > > >> > >>>>> This is the vote to release Apache MXNet (incubating) version > > >> 1.2.1. > > >> > >>>> Voting > > >> > >>>>> will start now and close Thursday June 21st 7:00 PM PDT. > > >> > >>>>> > > >> > >>>>> Link to release candidate 1.2.1.rc0: > > >> > >>>>> > > >> > >>>>> > > https://github.com/apache/incubator-mxnet/releases/tag/1.2.1.rc0 > > >> > >>>>> > > >> > >>>>> View this page for installation instructions: > > >> > >>>>> > > >> > >>>>> https://mxnet.incubator.apache.org/install/index.html > > >> > >>>>> > > >> > >>>>> (Note: The README.md points to the 1.2.1 tag and does not work > > at > > >> the > > >> > >>>>> moment). > > >> > >>>>> > > >> > >>>>> Please remember to test first before voting accordingly. > > >> > >>>>> > > >> > >>>>> +1 = approve > > >> > >>>>> +0 = no opinion > > >> > >>>>> -1 = disapprove (provide reason) > > >> > >>>>> > > >> > >>>>> Anirudh > > >> > >>>>> > > >> > >>>> > > >> > >>> > > >> > >> > > >> > > > >> > > > > > > > > >
