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

Plumbing for np.ma #1071

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Plumbing for np.ma #1071

merged 2 commits into from
Nov 14, 2023

Conversation

bryevdv
Copy link
Contributor

@bryevdv bryevdv commented Nov 6, 2023

This PR just adds minimal wrapping for the NumPy masked array module. Currently things will just fall back to numpy and emit the fallback warning:

Welcome to Legion Python interactive console
>>> import cunumeric as np
>>> x = np.ma.array(np.matrix([[1, 2], [3, 4]]), mask=[[0, 1], [1, 0]])
/home/bryan/work/cunumeric/cunumeric/coverage.py:186: RuntimeWarning: cuNumeric has not implemented numpy.ma.array and is falling back to canonical NumPy. You may notice significantly decreased performance for this function call.
  warnings.warn(
>>> x
masked_matrix(
  data=[[1, --],
        [--, 4]],
  mask=[[False,  True],
        [ True, False]],
  fill_value=999999)
>>> x.sum()	
/home/bryan/work/cunumeric/cunumeric/coverage.py:186: RuntimeWarning: cuNumeric has not implemented numpy.ma.core.MaskedArray.sum and is falling back to canonical NumPy. You may notice significantly decreased performance for this function call.
  warnings.warn(
5

and now calls hows up in coverage reporting as well

old311 ❯ CUNUMERIC_REPORT_COVERAGE=1 CUNUMERIC_REPORT_DUMP_CSV="out.csv" legate ~/tmp/cov.py
cuNumeric API coverage: 0/1 (0.0%)

~/work/cunumeric bv/ma*

old311 ❯ cat out.csv 
function_name,location,implemented
numpy.ma.array,/home/bryan/work/legate.core/_skbuild/linux-x86_64-3.11/cmake-build/_deps/legion-src/bindings/python/build/lib/legion_top.py:295,False

@bryevdv bryevdv added the category:task PR is a simple task and will not be included in release notes label Nov 6, 2023
@bryevdv bryevdv requested a review from manopapad November 6, 2023 17:30
@nv-legate nv-legate deleted a comment from copy-pr-bot bot Nov 6, 2023
@nv-legate nv-legate deleted a comment from copy-pr-bot bot Nov 8, 2023
@marcinz marcinz changed the base branch from branch-23.11 to branch-24.01 November 9, 2023 17:11
@bryevdv
Copy link
Contributor Author

bryevdv commented Nov 14, 2023

Looks like actual new mypy failures, I will fix before merging.

@ipdemes
Copy link
Contributor

ipdemes commented Nov 14, 2023

@bryevdv
I tested this branch with PSANA test that uses numpy.ma and got following warning:

/home/idemeshko/development/cunumeric_ma/cunumeric/coverage.py:186: RuntimeWarning: cuNumeric has not implemented numpy.ma.array and is falling back to canonical NumPy. You may notice significantly decreased performance for this function call.
  warnings.warn(
/home/idemeshko/development/cunumeric_ma/cunumeric/coverage.py:186: RuntimeWarning: cuNumeric has not implemented numpy.ma.median and is falling back to canonical NumPy. You may notice significantly decreased performance for this function call.

@bryevdv
Copy link
Contributor Author

bryevdv commented Nov 14, 2023

@ipdemes That's the expected behavior for now, AFAIK? This PR does not actually implement any of the masked array functionality in cunumeric/legate, just makes it so that masked array code runs at all (with fallback) and can report the coverage misses.

@ipdemes
Copy link
Contributor

ipdemes commented Nov 14, 2023

@bryevdv :by my comment I just wanted to confirm that this PR works as expected in one of the users code:)

@bryevdv
Copy link
Contributor Author

bryevdv commented Nov 14, 2023

Ah I misunderstood the intent, thanks for testing it out! 😄

@bryevdv bryevdv merged commit 1fa075d into nv-legate:branch-24.01 Nov 14, 2023
21 checks passed
@bryevdv bryevdv deleted the bv/ma branch November 14, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:task PR is a simple task and will not be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants