Skip to content

Conversation

flying-sheep
Copy link
Member

@flying-sheep flying-sheep commented Mar 6, 2025

Fixes #3

Intended properties:

  • like with Dask, functions taking cupy arrays or matrices should mostly return cupy arrays (except things like as_dense(to_memory=True) ofc
  • unlike with Dask, scalars should get returned as scalars, not arrays (no need to keep a single number in GPU memory)

Copy link

codecov bot commented Mar 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.45%. Comparing base (7319f89) to head (39a9da2).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
+ Coverage   96.87%   98.45%   +1.58%     
==========================================
  Files          15       15              
  Lines         256      259       +3     
==========================================
+ Hits          248      255       +7     
+ Misses          8        4       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

codspeed-hq bot commented Mar 6, 2025

CodSpeed Performance Report

Merging #51 will improve performances by ×2.9

Comparing pa/cupy (39a9da2) with main (7319f89)

Summary

⚡ 1 improvements
✅ 87 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_stats_benchmark[1-scipy.sparse.csr_array-float32-is_constant] 355.2 µs 123.9 µs ×2.9

@flying-sheep flying-sheep changed the title CyPy support CuPy support Mar 6, 2025
@flying-sheep flying-sheep added the run-gpu-ci Apply this label to run GPU CI once label Mar 10, 2025
@github-actions github-actions bot removed the run-gpu-ci Apply this label to run GPU CI once label Mar 10, 2025
@flying-sheep flying-sheep added run-gpu-ci Apply this label to run GPU CI once and removed run-gpu-ci Apply this label to run GPU CI once labels Mar 10, 2025
@flying-sheep flying-sheep marked this pull request as ready for review March 10, 2025 13:06
@flying-sheep flying-sheep added run-gpu-ci Apply this label to run GPU CI once and removed run-gpu-ci Apply this label to run GPU CI once labels Mar 10, 2025
@flying-sheep flying-sheep self-assigned this Mar 10, 2025
Copy link
Contributor

@ilan-gold ilan-gold left a comment

Choose a reason for hiding this comment

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

This PR seems to mostly just add types and a sum function? Is the reason that this all just "works" because cupy handles numpy well?

@github-actions github-actions bot removed the run-gpu-ci Apply this label to run GPU CI once label Mar 11, 2025
@flying-sheep flying-sheep added the run-gpu-ci Apply this label to run GPU CI once label Mar 11, 2025
@flying-sheep flying-sheep requested a review from ilan-gold March 11, 2025 09:05
@github-actions github-actions bot removed the run-gpu-ci Apply this label to run GPU CI once label Mar 11, 2025
@flying-sheep flying-sheep added the run-gpu-ci Apply this label to run GPU CI once label Mar 11, 2025
@github-actions github-actions bot removed the run-gpu-ci Apply this label to run GPU CI once label Mar 11, 2025
@flying-sheep flying-sheep added the run-gpu-ci Apply this label to run GPU CI once label Mar 11, 2025
@github-actions github-actions bot removed the run-gpu-ci Apply this label to run GPU CI once label Mar 11, 2025
Copy link
Contributor

@ilan-gold ilan-gold left a comment

Choose a reason for hiding this comment

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

In general I'm getting a bit concerned about all the casting going on but since it's in pre-commit, it should be manageable from a us-reviewing-other-PRs perspective i.e., we can always just tell people to look at the output. I was a little worried that there was no CI around this

@flying-sheep
Copy link
Member Author

Casting? You mean mypy? I think mypy and Python’s type system are simply not that great.

With TypeScript, I could rely on the type system saying the truth, but in Python it’s more often an error in the typing or a bug in mypy than my fault.

Makes the whole ordeal of rather limited use tbh if one can’t rely on the type system.

Maybe switching to PyRight could help.

@flying-sheep flying-sheep merged commit d47de41 into main Mar 11, 2025
13 of 15 checks passed
@flying-sheep flying-sheep deleted the pa/cupy branch March 11, 2025 14:42
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.

testing: add cupy
2 participants