Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

update with release notes for 1.4.0 release #13657

Merged
merged 28 commits into from
Dec 19, 2018
Merged

update with release notes for 1.4.0 release #13657

merged 28 commits into from
Dec 19, 2018

Conversation

srochel
Copy link
Contributor

@srochel srochel commented Dec 16, 2018

Description

update with release notes for 1.4.0 release

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http:https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@pengzhao-intel
Copy link
Contributor

Please wait a moment to merge the PR.
I will add the description of MKL-DNN parts tomorrow.

@pengzhao-intel
Copy link
Contributor

pengzhao-intel commented Dec 17, 2018

@srochel could you help add the below description into the new feature parts of r1.4?

@larroy @aaronmarkham could you help take a reveiw for the contents?

Thanks in advance.


MKDNN backend: Graph optimization and Quantization (experimental)

Two advanced features, graph optimization (operator fusion) and reduced-precision (INT8) computation, are introduced to MKL-DNN backend in this release (#12530, #13297, #13260).
These features significantly boost the inference performance on CPU (up to 4X) for a broad range of deep learning topologies.

  • Graph Optimization

MKL-DNN backend takes advantage of MXNet subgraph to implement the most of possible operator fusions for inference, such as Convolution + ReLU, Batch Normalization folding, etc. When using mxnet-mkl package, users can easily enable this feature by setting export MXNET_SUBGRAPH_BACKEND=MKLDNN.

  • Quantization

Performance of reduced-precision (INT8) computation is also dramatically improved after the graph optimization feature is applied on CPU Platforms. Various models are supported and can benefit from reduced-precision computation, including symbolic models, Gluon models and even custom models. Users can run most of the pre-trained models with only a few lines of commands and a new quantization script imagenet_gen_qsym_mkldnn.py. The observed accuracy loss is less than 0.5% for popular CNN networks, like ResNet-50, Inception-BN, MobileNet, etc.

Please find detailed information and performance/accuracy numbers here: MKLDNN README, quantization README and design proposal

NEWS.md Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Show resolved Hide resolved
addressed feedback from December 17 except ngraph related comments.
@srochel
Copy link
Contributor Author

srochel commented Dec 17, 2018

@pengzhao-intel added your change request.

Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some grammar and clarification and consistency updates in the overview sections.

The bug fix section seems like it could be more useful with some organization and clarity in the titles. I realize a lot of this is scraped and researching/updating is time consuming.

Some stuff in the big PR lists could be recategorized... even multi-categorized.

I don't think the JIRA tags are helpful in the descriptions in these release notes.

I wonder about keeping things in the notes that were reverted. Should these stay?

I think the test section has so much back and forth with enable/disable that it not helpful. A status would be nice - what's new, what's fixed, what's disabled now in this release.

NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some grammar and clarification and consistency updates in the overview sections.

The bug fix section seems like it could be more useful with some organization and clarity in the titles. I realize a lot of this is scraped and researching/updating is time consuming.

Some stuff in the big PR lists could be recategorized... even multi-categorized.

I don't think the JIRA tags are helpful in the descriptions in these release notes.

I wonder about keeping things in the notes that were reverted. Should these stay?

I think the test section has so much back and forth with enable/disable that it not helpful. A status would be nice - what's new, what's fixed, what's disabled now in this release.

@aaronmarkham
Copy link
Contributor

I kept getting nginx 405 errors from github when trying to submit this review (it was a lot of change requests >100) - it went through on the third try, so hopefully the review is all there.

@larroy
Copy link
Contributor

larroy commented Dec 17, 2018

Thanks for the corrections @srochel

@Roshrini
Copy link
Member

@mxnet-label-bot Add labels [pr-awaiting-response, Doc]

@marcoabreu marcoabreu added Doc pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Dec 17, 2018
aaronmarkham and others added 15 commits December 17, 2018 15:21
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
removed nGraph integration from 1.4.0 release notes
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Co-Authored-By: srochel <[email protected]>
Copy link
Contributor Author

@srochel srochel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adopting all of Aaron's change proposals.

NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
@sergeykolychev
Copy link
Contributor

@srochel Please let me know when I should merge this PR

NEWS.md Outdated Show resolved Hide resolved
srochel and others added 7 commits December 18, 2018 14:51
updates to address feedback from szha@, pengzhao-intel@ and TaoLv@
Addressed more of Aaron's comments and requests.
@srochel
Copy link
Contributor Author

srochel commented Dec 18, 2018

12/18 4pm - I addressed most of Aaron feedback. I will work with him tomorrow to resolve the remaining feedback from him. Issues from all other code reviewers are addressed.

Address all of Aaron's feedback, except question regarding Jira and Test section.
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a table of contents at the top.
I also reviewed the new MKLDNN, Graph optimization and quantization sections.

NEWS.md Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Show resolved Hide resolved
NEWS.md Show resolved Hide resolved
NEWS.md Show resolved Hide resolved
@srochel
Copy link
Contributor Author

srochel commented Dec 19, 2018

addressed all feedback as of 12/19 11.30am.

@sergeykolychev
Copy link
Contributor

@srochel after I get this merged into the 1.4.x branch, what's procedure to get it added to the master branch ?

* Add embedding to print_summary (#12796)
* Allow foreach on input with 0 length (#12471)
* [MXNET-360]auto convert str to bytes in img.imdecode when py3 (#10697)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add

@aaronmarkham aaronmarkham merged commit e413709 into apache:v1.4.x Dec 19, 2018
@szha
Copy link
Member

szha commented Dec 19, 2018

@aaronmarkham there was open comment by @astonzhang which I think should be addressed. What's your plan?

@aaronmarkham
Copy link
Contributor

Ah I didn't see Aston's comment. I can add that myself.

@astonzhang
Copy link
Member

Don't worry, it has already been added:
#13691

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Doc pr-awaiting-response PR is reviewed and waiting for contributor to respond
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet