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

Add test for mutually recursive remote functions. #5349

Merged
merged 5 commits into from
Aug 17, 2019

Conversation

robertnishihara
Copy link
Collaborator

@robertnishihara robertnishihara commented Aug 2, 2019

We can do this now that we export remote functions lazily.

Now that we export remote functions lazily, we don't seem to need the cloudpickle workaround anymore.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/15913/
Test PASSed.

@mehrdadn
Copy link
Contributor

mehrdadn commented Aug 6, 2019

(#453)

@robertnishihara
Copy link
Collaborator Author

As a note to myself, we may be able to move test_recursion.py inside of a normal pytest module in this PR.

@mehrdadn
Copy link
Contributor

mehrdadn commented Aug 6, 2019

Could you refresh my memory what the issue was? I've tried all sorts of examples and can't find a single one whose behavior changed with the workaround. I've tried older versions of CloudPickle as far as 0.5.3 (two years old).

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/16059/
Test FAILed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/16275/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/16285/
Test PASSed.

Copy link
Contributor

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

LGTM

python/ray/tests/test_basic.py Outdated Show resolved Hide resolved
python/ray/tests/test_basic.py Outdated Show resolved Hide resolved
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/16296/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/16297/
Test PASSed.

@robertnishihara robertnishihara merged commit bb31620 into ray-project:master Aug 17, 2019
@robertnishihara robertnishihara deleted the doublyrecursive branch August 17, 2019 07:46
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.

4 participants