Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement in kernel launches by using lighter NumPy APIs #258

Merged
merged 6 commits into from
Nov 14, 2019

Conversation

ybsh
Copy link
Collaborator

@ybsh ybsh commented Nov 7, 2019

I have replaced two heavy numpy function calls with lighter operations.
This reduced the train_mnist.py runtime by 20% (with the configuration shown here, noprof-nosync).

I have checked that clpy/tests/example_tests/test_gemm.py has passed.

Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 00a575a) passed in titanv.

Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 00a575a) passed in vega.

@ybsh
Copy link
Collaborator Author

ybsh commented Nov 7, 2019

@LWisteria I'd appreciate your review.

@ybsh
Copy link
Collaborator Author

ybsh commented Nov 8, 2019

The issue discussing the change: #257

Copy link
Member

@LWisteria LWisteria left a comment

Choose a reason for hiding this comment

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

@ybsh I reviewed. Change PR's title. It describes nothing now.

clpy/backend/function.pyx Outdated Show resolved Hide resolved
@LWisteria LWisteria assigned ybsh and unassigned LWisteria Nov 8, 2019
@ybsh ybsh changed the title 257 reduce numpy calls Performance improvement in kernel launches by using lighter NumPy APIs Nov 13, 2019
Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 99d45fe) failed in titanv.

============================= test session starts ==============================
platform linux -- Python 3.6.5, pytest-4.3.0, py-1.8.0, pluggy-0.13.0
rootdir: /home/clpy-jenkins-slave/workspace/clpy_testing_titan, inifile: setup.cfg
collected 0 items / 23 errors

==================================== ERRORS ====================================
_________ ERROR collecting tests/clpy_tests/core_tests/test_carray.py __________
test_carray.py:3: in <module>
    import clpy
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/backend/function.pxd:4: in init clpy.core.core
    ???
../../../clpy/backend/__init__.py:3: in <module>
    from clpy.backend import compiler  # NOQA
clpy/backend/function.pxd:4: in init clpy.backend.compiler
    ???
clpy/backend/function.pyx:38: in init clpy.backend.function
    ???
E   AttributeError: module 'clpy' has no attribute 'core'
__________ ERROR collecting tests/clpy_tests/core_tests/test_core.py ___________
test_core.py:3: in <module>
    import clpy
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/core/carray.pxi:14: in init clpy.core.core
    ???
E   AttributeError: type object 'clpy.core.core.Indexer' has no attribute '__reduce_cython__'
__ ERROR collecting tests/clpy_tests/core_tests/test_cupy_aliased_ndarray.py ___
test_cupy_aliased_ndarray.py:6: in <module>
    import clpy  # NOQA
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/core/carray.pxi:14: in init clpy.core.core
    ???
E   AttributeError: type object 'clpy.core.core.Indexer' has no attribute '__reduce_cython__'
_______ ERROR collecting tests/clpy_tests/core_tests/test_elementwise.py _______
test_elementwise.py:6: in <module>
    import clpy
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/core/carray.pxi:14: in init clpy.core.core

... and more 1221 lines

Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 99d45fe) failed in vega.

============================= test session starts ==============================
platform linux -- Python 3.6.5, pytest-4.3.0, py-1.8.0, pluggy-0.13.0
rootdir: /home/clpy-jenkins-slave/workspace/clpy_testing_vega, inifile: setup.cfg
collected 0 items / 23 errors

==================================== ERRORS ====================================
_________ ERROR collecting tests/clpy_tests/core_tests/test_carray.py __________
test_carray.py:3: in <module>
    import clpy
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/backend/function.pxd:4: in init clpy.core.core
    ???
../../../clpy/backend/__init__.py:3: in <module>
    from clpy.backend import compiler  # NOQA
clpy/backend/function.pxd:4: in init clpy.backend.compiler
    ???
clpy/backend/function.pyx:38: in init clpy.backend.function
    ???
E   AttributeError: module 'clpy' has no attribute 'core'
__________ ERROR collecting tests/clpy_tests/core_tests/test_core.py ___________
test_core.py:3: in <module>
    import clpy
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/core/carray.pxi:14: in init clpy.core.core
    ???
E   AttributeError: type object 'clpy.core.core.Indexer' has no attribute '__reduce_cython__'
__ ERROR collecting tests/clpy_tests/core_tests/test_cupy_aliased_ndarray.py ___
test_cupy_aliased_ndarray.py:6: in <module>
    import clpy  # NOQA
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/core/carray.pxi:14: in init clpy.core.core
    ???
E   AttributeError: type object 'clpy.core.core.Indexer' has no attribute '__reduce_cython__'
_______ ERROR collecting tests/clpy_tests/core_tests/test_elementwise.py _______
test_elementwise.py:6: in <module>
    import clpy
../../../clpy/__init__.py:17: in <module>
    from clpy import core  # NOQA
../../../clpy/core/__init__.py:1: in <module>
    from clpy.core import core  # NOQA
clpy/core/carray.pxi:14: in init clpy.core.core

... and more 1221 lines

Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 2e2f173) passed in titanv.

Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 2e2f173) passed in vega.

@ybsh
Copy link
Collaborator Author

ybsh commented Nov 13, 2019

@LWisteria I appreciate your review.

@ybsh ybsh assigned LWisteria and unassigned ybsh Nov 13, 2019
Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 53c1d5f) passed in vega.

Copy link

@jenkins-maekawa jenkins-maekawa left a comment

Choose a reason for hiding this comment

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

Test (commit 53c1d5f) passed in titanv.

Copy link
Member

@LWisteria LWisteria left a comment

Choose a reason for hiding this comment

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

LGTM

@LWisteria
Copy link
Member

@ybsh Can you report how the performance is improveed by this PR? I will merge after the report.

@LWisteria LWisteria assigned ybsh and unassigned LWisteria Nov 14, 2019
@ybsh
Copy link
Collaborator Author

ybsh commented Nov 14, 2019

I ran again train_mnist.py on

  • CuPy
  • ClPy before the change ( cc72c54)
  • ClPy after the change (82ad22b)

The configuration is the same as shown in this report.

The execution times (in seconds) are as follows.

cupy clpy-before clpy-after
51.21 78.05 63.73

These figures look slower overall compared to the report and I have not found the definitive cause with a quick investigation. Still, this is enough to show the modified ClPy runs faster.
This issue records what kind of overhead has been removed and how it was found.

@ybsh
Copy link
Collaborator Author

ybsh commented Nov 14, 2019

@LWisteria Please check my new report.

@ybsh ybsh assigned LWisteria and unassigned ybsh Nov 14, 2019
@LWisteria LWisteria merged commit 3a565e0 into clpy Nov 14, 2019
@LWisteria LWisteria deleted the 257-reduce_numpy_calls branch November 14, 2019 09:00
@LWisteria
Copy link
Member

LWisteria commented Nov 14, 2019

@ybsh Thank you for your report! This PR makes the difference from CuPy less than a half. Nice work!

@ybsh
Copy link
Collaborator Author

ybsh commented Nov 14, 2019

Thank you for merging.
And thank you for your advice!
@LWisteria @nsakabe-fixstars @vorj @ykitta-fixstars

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants