GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — feat: add map function by anna-charlotte · Pull Request #1187 · docarray/docarray

This repository was archived by the owner on Sep 9, 2026. It is now read-only.

Conversation

anna-charlotte commented Feb 28, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Goals:

Add map_docs function and map_batch:

docs = list(map_docs(da, lambda x : x, num_worker=4 ))

This will be different from doing

for doc in da:
    doc = f(doc)

leverage multiprocessing by benchmarking in test, check that using 2 CPUS is faster then using 1
map_docs()
map_docs_batch()
benchmarking tests
check and update documentation, if required. See guide

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

github-actions Bot added size/m area/core area/testing labels

Feb 28, 2023

anna-charlotte mentioned this pull request

Feb 28, 2023

Meta: DocArray v2 Roadmap #780

Closed

42 of 47 tasks

anna-charlotte linked an issue

Feb 28, 2023

that may be closed by this pull request

Map and apply with multiprocessing #1173

Closed

anna-charlotte force-pushed the feat-map-apply branch from 0c3bdfd to d72cbf5 Compare

March 1, 2023 08:53

github-actions Bot added area/cicd area/housekeeping area/setup labels

Mar 1, 2023

anna-charlotte force-pushed the feat-map-apply branch from 93be562 to d71de73 Compare

March 1, 2023 12:45

JohannesMessner commented Mar 1, 2023

Copy link

Copy Markdown

Member

Is apply_batched also in scope for this issue? And map/map_batched?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JohannesMessner reviewed

Mar 1, 2023

View reviewed changes

Comment thread

docarray/utils/apply.py Outdated

		func: Callable[[BaseDocument], BaseDocument],
		num_worker: Optional[int] = None,
		pool: Optional['Pool'] = None,
		show_progress: bool = False,

JohannesMessner Mar 1, 2023

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should we have a backend options to switch between multi-processing and multi-threading?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

anna-charlotte Mar 1, 2023

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

As far as I understood we only wanted to keep multiprocessing, not multithreading @samsja

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

samsja Mar 1, 2023

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@JoanFM maybe know more. But to me only multi processing makes sense. Can multi threading really improve performance here ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

JohannesMessner Mar 2, 2023

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Just the keep the PR discussion up to date with what we discussed on Discord: there will be multi threading since it makes sense for IO bound ops and tf/np/torch stuff

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anna-charlotte reacted with thumbs up emoji

All reactions

👍 1 reaction

anna-charlotte commented Mar 1, 2023

Copy link

Copy Markdown

Contributor Author

Is apply_batched also in scope for this issue? And map/map_batched?

Map is already included, but private right now. Not sure if we want to expose it again, or only one of the two.
The batched one, I can add in this PR, but right now I am still trying to get the benchmark to work correctly. Test is passing in the CI, but only because it is being skipped I think, because there in only 1 CPU available I think, therefore no performance comparison with 1 cpu vs 2 cpus.

JohannesMessner reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anna-charlotte force-pushed the feat-map-apply branch from 3a943a6 to 6c8176a Compare

March 2, 2023 08:45

github-actions Bot added component/array size/l and removed size/m labels

Mar 2, 2023

anna-charlotte force-pushed the feat-map-apply branch from 92d7a46 to 4c27409 Compare

March 2, 2023 14:59

samsja reviewed

Mar 2, 2023

View reviewed changes

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

anna-charlotte force-pushed the feat-map-apply branch from 9263bdb to 14e3cc1 Compare

March 2, 2023 16:42

JohannesMessner suggested changes

Mar 3, 2023

View reviewed changes

JohannesMessner left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good! We will make the map functions public, right?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

JohannesMessner suggested changes

Mar 3, 2023

View reviewed changes

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

anna-charlotte force-pushed the feat-map-apply branch from c7bef8f to a84b56d Compare

March 3, 2023 09:33

anna-charlotte commented Mar 3, 2023

Copy link

Copy Markdown

Contributor Author

Looks good! We will make the map functions public, right?

I dont really have an opinion on this. @samsja suggested to keep only one of the two exposed, map or apply. Why would you prefer this @samsja

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anna-charlotte marked this pull request as ready for review

March 3, 2023 09:38

anna-charlotte requested review from JohannesMessner and samsja

March 3, 2023 09:38

samsja reviewed

Mar 3, 2023

View reviewed changes

Comment thread

.github/workflows/ci.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

JohannesMessner commented Mar 3, 2023

Copy link

Copy Markdown

Member

Looks good! We will make the map functions public, right?

I dont really have an opinion on this. @samsja suggested to keep only one of the two exposed, map or apply. Why would you prefer this @samsja

Why should we keep only one @samsja? I think both make sense, the user might already have an in-place or pure function that they want to use without rewriting

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

samsja reviewed

Mar 3, 2023

View reviewed changes

Comment thread

docarray/utils/apply.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

anna-charlotte added 17 commits

March 3, 2023 14:30


          fix: add tests for func da to doc and da to other len da

18a377b

Signed-off-by: anna-charlotte <[email protected]>


          fix: revert last commit

245283f

Signed-off-by: anna-charlotte <[email protected]>


          test: add len assert

76fe8b7

Signed-off-by: anna-charlotte <[email protected]>


          test: add assertions

34b7f9c

Signed-off-by: anna-charlotte <[email protected]>


          test: add test to for da extend in batch apply

c7a968d

Signed-off-by: anna-charlotte <[email protected]>


          test: extend with only one doc

6cf8ed2

Signed-off-by: anna-charlotte <[email protected]>


          test: fix

5dc9e6d

Signed-off-by: anna-charlotte <[email protected]>


          fix: test

d3fc203

Signed-off-by: anna-charlotte <[email protected]>


          fix: test

45cdc4a

Signed-off-by: anna-charlotte <[email protected]>


          fix: set docs in apply

Signed-off-by: anna-charlotte <[email protected]>


          fix: indices

87a93ff

Signed-off-by: anna-charlotte <[email protected]>


          fix: indices

eeb7fae

Signed-off-by: anna-charlotte <[email protected]>


          fix: indices

72aaf21

Signed-off-by: anna-charlotte <[email protected]>


          fix: indices

c0f8029

Signed-off-by: anna-charlotte <[email protected]>


          fix:test

9b83c1f

Signed-off-by: anna-charlotte <[email protected]>


          fix: mypy

7638d86

Signed-off-by: anna-charlotte <[email protected]>


          fix: type hint

4a3a290

Signed-off-by: anna-charlotte <[email protected]>

anna-charlotte force-pushed the feat-map-apply branch from 6a8b753 to 4a3a290 Compare

March 3, 2023 13:30

anna-charlotte changed the title ~~feat: add apply function~~ feat: add map function

Mar 3, 2023

anna-charlotte added 2 commits

March 3, 2023 15:15


          fix: remove apply, only keep map

38aae7a

Signed-off-by: anna-charlotte <[email protected]>


          refactor: map to map_docs

01900c9

Signed-off-by: anna-charlotte <[email protected]>

samsja reviewed

Mar 3, 2023

View reviewed changes

Comment thread

docarray/utils/map.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

samsja reviewed

Mar 3, 2023

View reviewed changes

Comment thread

docarray/utils/map.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

anna-charlotte added 2 commits

March 3, 2023 15:51


          fix: apply suggestion

f6921e0

Signed-off-by: anna-charlotte <[email protected]>


          docs: add example usage

c3fb041

Signed-off-by: anna-charlotte <[email protected]>

github-actions Bot commented Mar 3, 2023

Copy link

Copy Markdown

📝 Docs are deployed on https://ft-feat-map-apply--jina-docs.netlify.app 🎉

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

samsja approved these changes

Mar 3, 2023

View reviewed changes

JohannesMessner approved these changes

Mar 3, 2023

View reviewed changes

anna-charlotte merged commit 6cd5912 into feat-rewrite-v2

Mar 3, 2023

anna-charlotte deleted the feat-map-apply branch

March 3, 2023 15:42

Reviewers

2 more reviewers

JohannesMessner

JohannesMessner approved these changes

samsja

samsja approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

area/cicd area/core area/housekeeping area/setup area/testing component/array size/l

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Map and apply with multiprocessing

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

anna-charlotte commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohannesMessner commented Mar 1, 2023

Uh oh!

JohannesMessner Mar 1, 2023

Choose a reason for hiding this comment

Uh oh!

anna-charlotte Mar 1, 2023

Choose a reason for hiding this comment

Uh oh!

samsja Mar 1, 2023

Choose a reason for hiding this comment

Uh oh!

JohannesMessner Mar 2, 2023

Choose a reason for hiding this comment

Uh oh!

anna-charlotte commented Mar 1, 2023

Uh oh!

Uh oh!

JohannesMessner left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anna-charlotte commented Mar 3, 2023

Uh oh!

Uh oh!

JohannesMessner commented Mar 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Mar 3, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anna-charlotte commented Feb 28, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

JohannesMessner left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.