Skip to content

DOC reference parallel_config instead of parallel_backend#1457

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

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

Conversation

@tomMoral

@tomMoral tomMoral commented Jun 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

codecov Bot commented Jun 22, 2023 •
edited
Loading

Copy link
Copy Markdown

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
Comment thread examples/parallel/distributed_backend_simple.py Outdated
Comment thread examples/serialization_and_wrappers.py
Comment thread joblib/test/test_config.py Outdated
Comment thread joblib/test/test_config.py Outdated
Comment thread joblib/test/test_config.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread joblib/_dask.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread joblib/_dask.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread joblib/test/test_dask.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread joblib/test/test_dask.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
Comment thread joblib/test/test_dask.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread joblib/test/test_dask.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread joblib/test/test_dask.py Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread doc/parallel.rst Outdated
tomMoral
tomMoral commented Jun 23, 2023
View reviewed changes
Comment thread doc/parallel.rst
jeremiedbb
jeremiedbb approved these changes Jun 23, 2023
View reviewed changes

@jeremiedbb jeremiedbb left a comment

Copy link
Copy Markdown
Contributor

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

@ogrisel ogrisel left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread doc/parallel.rst Outdated
Comment thread joblib/test/test_config.py
Comment thread joblib/test/test_config.py Outdated
@ogrisel

ogrisel commented Jun 26, 2023

Copy link
Copy Markdown
Contributor

@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

ogrisel commented Jun 27, 2023

Copy link
Copy Markdown
Contributor

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

@ogrisel

ogrisel commented Jun 27, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

ogrisel commented Jun 27, 2023

Copy link
Copy Markdown
Contributor

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

ogrisel commented Jun 27, 2023

Copy link
Copy Markdown
Contributor

@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

tomMoral commented Jun 27, 2023

Copy link
Copy Markdown
Contributor Author

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

ogrisel commented Jun 28, 2023

Copy link
Copy Markdown
Contributor

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