Skip to content

add back the iteration based seeding for Array #634

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KristofferC
Copy link
Collaborator

These were removed in #472 and while I was a bit uncomfortable with it I did not protest. However, now after doing some benchmarks I see that this has a runtime cost that is very much nonsignificant.

Below is a profile and the big peaks are from seed!.

gnome-shell-screenshot-57lfk9

If I use the iteration-based seed function they completely disappear. In order to not break anything, I added these back but with a restriction to Array so that e.g. GPU arrays will keep using the broadcast based seeding.

@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.32 🎉

Comparison is base (1592fe9) 87.28% compared to head (bafdb3b) 87.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #634      +/-   ##
==========================================
+ Coverage   87.28%   87.60%   +0.32%     
==========================================
  Files          10       10              
  Lines         912      936      +24     
==========================================
+ Hits          796      820      +24     
  Misses        116      116              
Impacted Files Coverage Δ
src/apiutils.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@devmotion
Copy link
Member

I wonder if one could use map! instead of broadcasting? I would assume that this would still be GPU-compatible but possibly there is (almost) no runtime cost for Array?

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

Successfully merging this pull request may close these issues.

2 participants