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

Use Distributed.pmap in ParallelTableTransform and ColwiseFeatureTransform #288

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

eliascarv
Copy link
Member

closes #286

@eliascarv eliascarv requested a review from juliohm June 24, 2024 13:01
@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.93%. Comparing base (56e0add) to head (4893c3c).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #288      +/-   ##
==========================================
- Coverage   94.95%   94.93%   -0.02%     
==========================================
  Files          48       48              
  Lines        1388     1383       -5     
==========================================
- Hits         1318     1313       -5     
  Misses         70       70              

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

@juliohm
Copy link
Member

juliohm commented Jun 25, 2024

We need to benchmark the change, compare it with the previous code. Make sure that pmap is using the CachePool if needed. I know it is the default in latest Julia versions, please double check if Julia v1.9 has it as the default.

@juliohm
Copy link
Member

juliohm commented Jun 25, 2024

Additionally, we use tcollect in ColwiseTransform, and should probably use a pmap too. If that is indeed the case, please update the code, drop Transducers.jl from the list of deps, and update the documentation accordingly.

@eliascarv eliascarv changed the title Use Distributed.pmap in ParallelTableTransform Use Distributed.pmap in ParallelTableTransform and ColwiseFeatureTransform Jun 25, 2024
@juliohm juliohm merged commit 94ae9b2 into master Jun 25, 2024
8 checks passed
@juliohm juliohm deleted the parallel branch June 25, 2024 13:06
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.

Use Distributed processes in ParallelTableTransforms
3 participants