Re: [DISCUSS] Improve code quality

2019-09-14 Thread Wang Wei
Hi Moaz,

I agree with you.
We did take some efforts to improve the code quality.
[1] introduces some tools for enforcing the coding style.
[2] introduces some tools for adding documentations.

The current issue is that our contributors may have applied different
coding styles and tools using their different editors.
I suggest to do some tests during the CI process, e.g., running the cpplint
and pylint.
If all tests pass, then we merge the PR.

Thanks!

Best,
Wei

[1]. http://singa.apache.org/en/develop/contribute-code.html
[2]. http://singa.apache.org/en/develop/contribute-docs.html

On Sun, Sep 15, 2019 at 2:36 AM Moaz Reyad  wrote:

> Dear team,
>
> Since SINGA is going to graduate soon from the incubator, I propose to use
> some tools to ensure high code quality. These tools check for known
> problems in the code and provide a detailed report for fixing them. May be
> some code came from scientific experimental projects. We need to improve
> this code according to industry standards, so it can be used with more real
> life projects.
>
> 1. I propose to add the code quality tools (cpplint[1], pylint[2] and
> lgtm[3]) to SINGA contribution guideline[4], so that each developer is
> encouraged to install and run code quality checks in his local repo and fix
> any problems before creating a pull request.
>
> 1.A CPP Lint: running cpplint in the src directory shows 822 errors, while
> running in the include directory shows 708 errors. The guidelines [4] has
> an outdated information that instructs developers to use an old
> non-existing file tool/cpplint.py.
>
> 1.B Python Lint: running pylint in python/singa shows 5.00/10 rating, while
> running in python/rafiki shows 0.00/10 rating.
>
> 1.C LGTM :There is a Jira ticket for adding LGTM badges to the README[5],
> so the quality of the code becomes more clear to everyone. LGTM pull
> request automation can't be enabled in Apache repo due to infra
> restrictions[6], but it works on personal forks of the project. Currently
> LGTM rates both C++ and Python code in SINGA as grade D.
>
> 2. I propose also to give the code quality higher priority in the next
> release since it is probably going to be the first release after
> graduation. The team is invited to fix as much as possible from the current
> code issues and to use tools that check their new code before pushing it to
> SINGA. Let's try to make the lgtm grade and lint rating as high as
> possible.
>
> Improving code quality is required to attract new users and developers.
> Users will trust more the project with better code and developers will be
> happy to contribute to it. It will also make the code review process easier
> and more productive instead of wasting time in finding and fixing known
> code problems.
>
> New developers (or old developers who did not contribute for a while and
> would like to warm up) can start working on fixing lgtm and lint issues,
> since they are usually easy and there is a clear explanation of the problem
> and how to solve it.
>
> What do you think?
>
> p.s. This discussion is the first topic in a series of proposals to improve
> SINGA as it will be an Apache top level project soon. The next proposal
> will discuss improving the build and test pipeline in a separate thread to
> avoid discussing too many things in one thread.
>
> best regards,
> Moaz
>
> [1] https://pypi.org/project/cpplint/
> [2] https://www.pylint.org/
> [3] https://lgtm.com/
> [4] http://singa.apache.org/develop/contribute-code.html
> [5] https://issues.apache.org/jira/projects/SINGA/issues/SINGA-484
> [6] https://issues.apache.org/jira/browse/INFRA-17954
>


[DISCUSS] Improve code quality

2019-09-14 Thread Moaz Reyad
Dear team,

Since SINGA is going to graduate soon from the incubator, I propose to use
some tools to ensure high code quality. These tools check for known
problems in the code and provide a detailed report for fixing them. May be
some code came from scientific experimental projects. We need to improve
this code according to industry standards, so it can be used with more real
life projects.

1. I propose to add the code quality tools (cpplint[1], pylint[2] and
lgtm[3]) to SINGA contribution guideline[4], so that each developer is
encouraged to install and run code quality checks in his local repo and fix
any problems before creating a pull request.

1.A CPP Lint: running cpplint in the src directory shows 822 errors, while
running in the include directory shows 708 errors. The guidelines [4] has
an outdated information that instructs developers to use an old
non-existing file tool/cpplint.py.

1.B Python Lint: running pylint in python/singa shows 5.00/10 rating, while
running in python/rafiki shows 0.00/10 rating.

1.C LGTM :There is a Jira ticket for adding LGTM badges to the README[5],
so the quality of the code becomes more clear to everyone. LGTM pull
request automation can't be enabled in Apache repo due to infra
restrictions[6], but it works on personal forks of the project. Currently
LGTM rates both C++ and Python code in SINGA as grade D.

2. I propose also to give the code quality higher priority in the next
release since it is probably going to be the first release after
graduation. The team is invited to fix as much as possible from the current
code issues and to use tools that check their new code before pushing it to
SINGA. Let's try to make the lgtm grade and lint rating as high as possible.

Improving code quality is required to attract new users and developers.
Users will trust more the project with better code and developers will be
happy to contribute to it. It will also make the code review process easier
and more productive instead of wasting time in finding and fixing known
code problems.

New developers (or old developers who did not contribute for a while and
would like to warm up) can start working on fixing lgtm and lint issues,
since they are usually easy and there is a clear explanation of the problem
and how to solve it.

What do you think?

p.s. This discussion is the first topic in a series of proposals to improve
SINGA as it will be an Apache top level project soon. The next proposal
will discuss improving the build and test pipeline in a separate thread to
avoid discussing too many things in one thread.

best regards,
Moaz

[1] https://pypi.org/project/cpplint/
[2] https://www.pylint.org/
[3] https://lgtm.com/
[4] http://singa.apache.org/develop/contribute-code.html
[5] https://issues.apache.org/jira/projects/SINGA/issues/SINGA-484
[6] https://issues.apache.org/jira/browse/INFRA-17954


[GitHub] [incubator-singa] joddiy commented on issue #462: SINGA-333 Add ONNX backend test

2019-09-14 Thread GitBox
joddiy commented on issue #462: SINGA-333 Add ONNX backend test
URL: https://github.com/apache/incubator-singa/pull/462#issuecomment-531490830
 
 
   > But if we can't run onnx backend test cases, then we simply can't claim 
that we support onnx backend.
   > 
   > We don't have to pass all the test cases, but at least some of them should 
pass. Currently [all of the test cases 
fail](https://issues.apache.org/jira/projects/SINGA/issues/SINGA-458), which 
can be understood as if the current support of onnx backend is 0%.
   
   Hi, moazreyad, I know what you mean. Please check the link 
[here](https://github.com/apache/incubator-singa/pull/534). I mean you cannot 
just easily call all the onnx test cases, it cannot run. I have adapted the 
onnx backend test cases from onnx there, and discard the cases that we cannot 
support.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-singa] moazreyad commented on issue #462: SINGA-333 Add ONNX backend test

2019-09-14 Thread GitBox
moazreyad commented on issue #462: SINGA-333 Add ONNX backend test
URL: https://github.com/apache/incubator-singa/pull/462#issuecomment-531487825
 
 
   But if we can't run onnx backend test cases, then we simply can't claim that 
we support onnx backend.
   
   We don't have to pass all the test cases, but at least some of them should 
pass. Currently [all of the test cases 
fail](https://issues.apache.org/jira/projects/SINGA/issues/SINGA-458), which 
can be understood as if the current support of onnx backend is 0%.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (SINGA-486) September report

2019-09-14 Thread Moaz Reyad (Jira)


[ 
https://issues.apache.org/jira/browse/SINGA-486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16929778#comment-16929778
 ] 

Moaz Reyad commented on SINGA-486:
--

It was [requested in the mail 
list|https://lists.apache.org/thread.html/6509d10c02817d662ebe04c0f7415ee38a2033861a6e23653a5a8a97@%3Cdev.singa.apache.org%3E]
 that the reports should have more details. I propose that we open the ticket 
for the report as early as possible, so we have more time to fill it with 
useful information. 

For example, if next report is in December (assuming reporting cycle does not 
change after graduation), we can open a ticket for December report from now. 
The ticket can be updated with every news or update that happens in the project 
from now to December. So when it is time for submitting the report, it will be 
ready and complete. 

Also this issue and SINGA-461 can be closed now.

> September report
> 
>
> Key: SINGA-486
> URL: https://issues.apache.org/jira/browse/SINGA-486
> Project: Singa
>  Issue Type: Task
>Reporter: wangwei
>Priority: Major
>
> Here is a draft for the report, which is also available at 
> https://wiki.apache.org/confluence/display/INCUBATOR/September2019
>  
> --
> h2. 
> [SINGA|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#singa]
> SINGA is a distributed deep learning platform.
> SINGA has been incubating since 2015-03-17.
> h3. [Three most important unfinished issues to address before 
> graduating:|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#three-most-important-unfinished-issues-to-address-before-graduating-15]
>  # Fix the code grant issue being discussed in the general@ list.
>  # Update the documentation and packages (e.g., docker images and conda 
> pacakges)
>  # 
> h3. [Are there any issues that the IPMC or ASF Board need to be aware 
> of?|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#are-there-any-issues-that-the-ipmc-or-asf-board-need-to-be-aware-of-15]
> N.A
> h3. [How has the community developed since the last 
> report?|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#how-has-the-community-developed-since-the-last-report-15]
>  * The community has been discussing about the graduation
>  * Currently, we have 45 contributors, 1803 stars and 479 forks on github.
> h3. [How has the project developed since the last 
> report?|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#how-has-the-project-developed-since-the-last-report-15]
>  * Since the last report, we have been focusing on enhancing the ONNX feature 
> and distributed training. More than 20 ONNX operators are added. Distributed 
> training using NCCL and MPI has been tested.
>  * 180 new commits are merged since last report.
>  * There are 57, 142 and 472 emails on dev@ list for June, July and August 
> respectively.
> h3. [How would you assess the podling's 
> maturity?|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#how-would-you-assess-the-podlings-maturity-15]
> Please feel free to add your own commentary.
>  *  Initial setup
>  *  Working towards first release
>  *  Community building
>  *  Nearing graduation
>  *  Other:
> h3. [Date of last 
> release:|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#date-of-last-release-15]
> 2019-04-20
> h3. [When were the last committers or PPMC members 
> elected?|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#when-were-the-last-committers-or-ppmc-members-elected-15]
> 2018-12-21
> h3. [Have your mentors been helpful and 
> responsive?|https://wiki.apache.org/confluence/display/INCUBATOR/September2019#have-your-mentors-been-helpful-and-responsive-15]
> Are things falling through the cracks? If so, please list any open issues 
> that need to be addressed.
> Yes. The mentors raised very important issues like the code grant in the 
> graduation discussion.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)


[GitHub] [incubator-singa] moazreyad commented on a change in pull request #529: SINGA-484 Code analysis with LGTM

2019-09-14 Thread GitBox
moazreyad commented on a change in pull request #529: SINGA-484 Code analysis 
with LGTM
URL: https://github.com/apache/incubator-singa/pull/529#discussion_r324426747
 
 

 ##
 File path: include/singa/io/transformer.h
 ##
 @@ -56,12 +56,12 @@ class ImageTransformer: public Transformer {
 
   Tensor Apply(int flag, Tensor& input) override;
 
-  const bool featurewise_center() const { return featurewise_center_; }
-  const bool featurewise_std_norm() const { return featurewise_std_norm_; }
-  const bool horizontal_mirror() const { return horizontal_mirror_; }
-  const int resize_height() const { return resize_height_; }
-  const int resize_width() const { return resize_width_; }
-  const float rescale() const { return rescale_; }
+  bool featurewise_center() const { return featurewise_center_; }
 
 Review comment:
   Yes, LGTM suggested to the remove the const because it is useless. More 
information can be found [here](https://lgtm.com/rules/2156200647/). 
   
   There are more alerts that I could try to fix, but because the [testing 
pipeline is not working](https://issues.apache.org/jira/browse/SINGA-488), I 
can't be sure that my fixes will not cause other problems.
   
   All the alerts can be found in this 
[link](https://lgtm.com/projects/g/apache/incubator-singa/alerts/?mode=list), 
which is also available in the Jira ticket 
[484](https://issues.apache.org/jira/browse/SINGA-484)
   
   In this pull request, I propose to add the code analysis badges to SINGA 
readme and I fixed few alerts. But there are many more to be fixed. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services