Skip to content

DOC reference parallel_config instead of parallel_backend#1457

Merged
tomMoral merged 19 commits intojoblib:masterfrom
tomMoral:DOC_parallel_config
Jun 27, 2023
Merged

DOC reference parallel_config instead of parallel_backend#1457
tomMoral merged 19 commits intojoblib:masterfrom
tomMoral:DOC_parallel_config

Conversation

@tomMoral
Copy link
Copy Markdown
Contributor

@tomMoral tomMoral commented Jun 22, 2023 •
edited
Loading

Follow up for #1451 to use parallel_config as much as possible in examples and in the tests.

In doing so, I realized that it actually has a different behavior than parallel_backend.
This PR does:

  • make it possible to specify the default_n_jobs per backend (in particular to have default n_jobs=-1 for dask).
  • Fix the nested parallel_config usage. Calling the two following examples is equivalent while the first one would give LokyBackend with 2 jobs with the current version on master.
with parallel_config(backend='threading'):
    with parallel_config(n_jobs=2):
        p = Parallel()

with parallel_config(backend='loky', n_jobs=2):
        p = Parallel()

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 22, 2023 •
edited
Loading

Codecov Report

Patch coverage: 98.13% and project coverage change: -0.10 ⚠️

Comparison is base (2edd849) 94.93% compared to head (ddcbebe) 94.83%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1457      +/-   ##
==========================================
- Coverage   94.93%   94.83%   -0.10%     
==========================================
  Files          44       45       +1     
  Lines        7439     7471      +32     
==========================================
+ Hits         7062     7085      +23     
- Misses        377      386       +9
Impacted Files Coverage Δ
joblib/test/test_dask.py 91.19% <96.15%> (ø)
joblib/test/test_config.py 97.59% <97.59%> (ø)
joblib/_dask.py 92.71% <100.00%> (+0.03%) ⬆️
joblib/_parallel_backends.py 93.47% <100.00%> (+0.02%) ⬆️
joblib/parallel.py 95.90% <100.00%> (-1.34%) ⬇️
joblib/test/test_parallel.py 96.11% <100.00%> (-0.09%) ⬇️

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

@tomMoral tomMoral requested a review from jeremiedbb June 22, 2023 16:30
jeremiedbb
jeremiedbb reviewed Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
tomMoral and others added 3 commits June 23, 2023 13:43
@tomMoral @jeremiedbb
Apply suggestions from code review
8b4808f 
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
@tomMoral
ENH non-recursive config
f05c4d1
@tomMoral
FIX backend reset in nested config
8658960
jeremiedbb
jeremiedbb approved these changes Jun 23, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

LGTM

ogrisel
ogrisel reviewed Jun 23, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

I did not have time to fully review the PR carefully but what I read looks good.

tomMoral and others added 3 commits June 26, 2023 09:30
@tomMoral @ogrisel
Apply suggestions from code review
71c159b 
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@tomMoral @ogrisel
Update doc/parallel.rst
b1bbeb2 
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@tomMoral
FIX intersphinx localization
4354e6d
@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Jun 26, 2023

@tomMoral I do not understand where the doc problems come from when reading the readthedocs output. Do you get more informative feedback when building the doc locally?

Readthedocs is also failing on unrelated PRs (e.g. #1463) but it would be great to fix it here to see the updated doc for the parallel_config API.

@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Jun 27, 2023

I tried locally and the output folders stay mostly empty but there is no error message and the return code is 0...

@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Jun 27, 2023 •
edited
Loading

So the sphinx crash is probably caused by 2 offending examples:

  • examples/serialization_and_wrappers.py
  • examples/parallel_random_state.py (only when ran via sphinx, it works fine otherwise...)

Deleting both example files makes the sphinx generation complete successfully.

But those failures are not related to this PR. I think they also fail on master and even with joblib 1.2.0 but I want to double check.

EDIT: I opened #1465 for the first problem.

@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Jun 27, 2023

Actually the above comment is macos only. On the output of the CI it fails because of a warning:

/home/docs/checkouts/readthedocs.org/user_builds/joblib/envs/1457/lib/python3.7/site-packages/joblib/parallel.py:docstring of joblib.parallel.parallel_config:1: WARNING: duplicate object description of joblib.parallel_config, other instance in generated/joblib.parallel_config, use :noindex: for one of them

@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Jun 27, 2023

@tomMoral I added the :noindex: and it fixed the problem and the function is still indexed and reachable from the right location in the left navigation menu:

https://joblib--1457.org.readthedocs.build/en/1457/generated/joblib.parallel_config.html

@tomMoral
Copy link
Copy Markdown
Contributor Author

tomMoral commented Jun 27, 2023

Yeah! thanks @ogrisel :)

I don't think the broken build on sklearn is due to this PR as this seems to be an update on scipy\ so merging this one :)

@tomMoral tomMoral merged commit 5d88860 into joblib:master Jun 27, 2023
@tomMoral tomMoral deleted the DOC_parallel_config branch June 27, 2023 22:55
@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Jun 28, 2023

Indeed, i think the scipy solver problem was fixed in scikit-learn main branch and will be released as part of 1.3.

@larsoner larsoner mentioned this pull request Jun 30, 2023
Closed
@mscheltienne mscheltienne mentioned this pull request Jul 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ogrisel ogrisel ogrisel left review comments

@jeremiedbb jeremiedbb jeremiedbb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tomMoral @ogrisel @jeremiedbb