Skip to content

Add basic cunumeric.patch module#225

Merged
bryevdv merged 1 commit intonv-legate:branch-22.03from
bryevdv:bryanv/patch
Mar 22, 2022
Merged

Add basic cunumeric.patch module#225
bryevdv merged 1 commit intonv-legate:branch-22.03from
bryevdv:bryanv/patch

Conversation

@bryevdv
Copy link
Contributor

@bryevdv bryevdv commented Mar 21, 2022

This PR adds a very basic cunumeric.patch module that can be imported early in a script to "automatically" replace NumPy with cuNumeric. With the following example script:

import cunumeric.patch

import numpy as np

a = np.random.randn(4, 5)

np.argmin(a, axis=0)
np.argmin(a, axis=1)

np.argmax(a, axis=0)
np.argmax(a, axis=1)

The desired coverage results are generated:

~/work/cunumeric branch-22.03*
legate37 ❯ legate /tmp/foo.py  -cunumeric:report:coverage 
cuNumeric API coverage: 5/5 (100.0%)

Notes

  • This is not yet quite "zero-change", but is a necessary start, and can be utilized immediately, with one line of code added. Assuming a -m option is implemented in legion in the future, the it should become possible to update cunumeric patch with a "main" that affords this usage via the command line:

    legate -m cunumeric.patch foo.py
    

    without making any source changes at all.

  • True "zero-change" would require changes to legate but there would be questions to answer about having to make legate know about cunumeric.

  • The patching is very simple-minded, and may have edge-cases or shortcomings. E.g. I had hoped to be able to warn loudly in case numpy was already imported, but cunumeric itself imports numpy, so that is not feasible. We cannot rely on the presence or absence of numpy in sys.modules to condition a warning. I would recommend not widely promoting this module, and keeping it intended as a tool for experienced dev-rel for this reason.

@bryevdv bryevdv requested a review from magnatelee March 21, 2022 20:53
@magnatelee
Copy link
Contributor

LGTM. Feel free to merge this. (I believe the failures have nothing to do with this PR.)

@bryevdv bryevdv merged commit 5ab23a3 into nv-legate:branch-22.03 Mar 22, 2022
@bryevdv bryevdv deleted the bryanv/patch branch March 22, 2022 04:17
ipdemes pushed a commit to ipdemes/cunumeric that referenced this pull request Jun 7, 2022
updates:
- [github.com/pre-commit/mirrors-clang-format: v14.0.1 → v14.0.3](pre-commit/mirrors-clang-format@v14.0.1...v14.0.3)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
manopapad pushed a commit that referenced this pull request Nov 17, 2024
* fix compilation errors with cuda_help.h

* cuda_help.h: wrap check_* in namespace cunumeric

* fix cuda_help: add cunumeric:: in the macro

---------

Co-authored-by: Wonchan Lee <wonchanl@nvidia.com>
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.

2 participants