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

[Core][multiprocessing] Make Pool.imap() and Pool.imap_unordered() throw TypeError on non-iterable/iterator #24237

Closed
simontindemans opened this issue Apr 26, 2022 · 7 comments · Fixed by #31799
Assignees
Labels
bug Something that is supposed to be working; but isn't core Issues that should be addressed in Ray Core P0 Issues that should be fixed in short order size:small
Milestone

Comments

@simontindemans
Copy link
Contributor

Description

Currently, the Ray implementations of Pool.imap() and Pool.imap_unordered() accept arguments of the type (func, iterable), where iterable can be an actual iterable, an iterator, or a single input. In In the letter case, the IMapIterator constructor treats iterable as [iterable]. The code implementing this (since this PR, which is functionally identical to previous versions in this regard) is as follows:

        try:
            self._iterator = iter(iterable)
        except TypeError:
            # for compatibility with prior releases, encapsulate non-iterable in a list
            iterable = [iterable]
            self._iterator = iter(iterable)

This behaviour deviates from the core Python implementations of map and multiprocessing.imap and multiprocessing.imap_unordered, which all throw a TypeError when given a single value as iterable.

As this potentially breaks existing code, this was proposed as an issue to be addressed in a future release here:
Originally posted by @edoakes in #24117 (comment)

Use case

The aim is to have a consistent interface for:

  • Python map
  • multiprocessing: imap and imap_unordered
  • ray.util.multiprocessing: imap and imap_unordered
@simontindemans simontindemans added the enhancement Request for new feature and/or capability label Apr 26, 2022
@simontindemans
Copy link
Contributor Author

Minimal reproducing code:

import ray.util.multiprocessing as raymp
import multiprocessing as mp

def f(x):
    print(x*x)

if __name__ == "__main__":

    # standard python map
    # works
    [value for value in map(f, [3])]
    # TypeError
    try:
        [value for value in map(f, 3)]
    except TypeError:
        print("TypeError as expected")

    # multiprocessing imap
    pool = mp.Pool()
    # works
    [value for value in pool.imap(f, [3])]
    # TypeError
    try:
        [value for value in pool.imap(f, 3)]
    except TypeError:
        print("TypeError as expected")
    pool.terminate()

    # ray multiprocessing imap
    pool = raymp.Pool()
    # works
    [value for value in pool.imap(f, [3])]
    # No TypeError
    [value for value in pool.imap(f, 3)]
    print("WARNING: should throw TypeError") 
    pool.terminate() 

@jjyao
Copy link
Collaborator

jjyao commented Apr 26, 2022

We can do it as part of 2.0 if we decide to do it.

@jjyao jjyao added this to the Core Backlog milestone Apr 26, 2022
@edoakes
Copy link
Contributor

edoakes commented Apr 26, 2022

@jjyao do we have a label tracking items for 2.0?

@jjyao
Copy link
Collaborator

jjyao commented Apr 26, 2022

@edoakes Not yet. Going to create one.

@jjyao jjyao added the Ray 2.0 label Apr 26, 2022
@simontindemans
Copy link
Contributor Author

Perfect; you can ping me if/when you'd like to make the change. It's fairly trivial: removing a four-line workaround I had to insert to maintain compatibility.

@rkooo567 rkooo567 added bug Something that is supposed to be working; but isn't P1 Issue that should be fixed within a few weeks and removed enhancement Request for new feature and/or capability labels Jul 8, 2022
@ericl ericl removed the Ray 2.0 label Jul 28, 2022
@richardliaw richardliaw added the core Issues that should be addressed in Ray Core label Oct 7, 2022
@hora-anyscale
Copy link
Contributor

@simontindemans - if you could share a PR that fixes this it would be very much appreciated.

@rkooo567 rkooo567 added P0 Issues that should be fixed in short order and removed P1 Issue that should be fixed within a few weeks labels Jan 11, 2023
rkooo567 pushed a commit that referenced this issue Jan 23, 2023
…nd imap_unordered methods of ray.util.multiprocessing.Pool (#31845)

Context: #24237

We will raise this warning in Ray 2.3. For Ray 2.4, we will merge #31799 which causes a TypeError to be raised instead.
@rkooo567
Copy link
Contributor

We will send a warning in 2.3, raise an exception at 2.4 cc @cadedaniel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to be working; but isn't core Issues that should be addressed in Ray Core P0 Issues that should be fixed in short order size:small
Projects
None yet
8 participants