Skip to content

Commit

Permalink
[ENH] reactivate and fix test_multiprocessing_idempotent (#5573)
Browse files Browse the repository at this point in the history
This PR fixes two untracked issues:

* `test_multiprocessing_idempotent` would be skipped. A comment said
that some issues were related to it, but there was no GitHub issue. This
PR removes the skip, checking whether the problem is gone.
* `test_multiprocessing_idempotent` would also fail for forecasters with
probabilistic capability, due to the output type not being comparable
via the approx array comparison. The modified test now skips for
`predict_proba` outputs.

The failures would also be thrown when using `check_estimator`, this was
noticed by @benHeid for `ConformalIntervals`.
  • Loading branch information
fkiraly committed Dec 22, 2023
1 parent 7336387 commit c9152b1
Showing 1 changed file with 32 additions and 24 deletions.
56 changes: 32 additions & 24 deletions sktime/tests/test_all_estimators.py
Expand Up @@ -1428,8 +1428,6 @@ def test_save_estimators_to_file(
err_msg=msg,
)

# todo: this needs to be diagnosed and fixed - temporary skip
@pytest.mark.skip(reason="hangs on mac and unix remote tests")
def test_multiprocessing_idempotent(
self, estimator_instance, scenario, method_nsc_arraylike
):
Expand All @@ -1443,29 +1441,39 @@ def test_multiprocessing_idempotent(
method_nsc = method_nsc_arraylike
params = estimator_instance.get_params()

if "n_jobs" in params:
# run on a single process
# -----------------------
estimator = deepcopy(estimator_instance)
estimator.set_params(n_jobs=1)
set_random_state(estimator)
result_single_process = scenario.run(
estimator, method_sequence=["fit", method_nsc]
)
# test runs only if n_jobs is a parameter of the estimator
if "n_jobs" not in params:
return None

# run on multiple processes
# -------------------------
estimator = deepcopy(estimator_instance)
estimator.set_params(n_jobs=-1)
set_random_state(estimator)
result_multiple_process = scenario.run(
estimator, method_sequence=["fit", method_nsc]
)
_assert_array_equal(
result_single_process,
result_multiple_process,
err_msg="Results are not equal for n_jobs=1 and n_jobs=-1",
)
# skip test for predict_proba
# this produces a BaseDistribution object, for which no ready
# equality check is implemented
if method_nsc == "predict_proba":
return None

# run on a single process
# -----------------------
estimator = deepcopy(estimator_instance)
estimator.set_params(n_jobs=1)
set_random_state(estimator)
result_single_process = scenario.run(
estimator, method_sequence=["fit", method_nsc]
)

# run on multiple processes
# -------------------------
estimator = deepcopy(estimator_instance)
estimator.set_params(n_jobs=-1)
set_random_state(estimator)
result_multiple_process = scenario.run(
estimator, method_sequence=["fit", method_nsc]
)

_assert_array_equal(
result_single_process,
result_multiple_process,
err_msg="Results are not equal for n_jobs=1 and n_jobs=-1",
)

def test_dl_constructor_initializes_deeply(self, estimator_class):
"""Test DL estimators that they pass custom parameters to underlying Network."""
Expand Down

0 comments on commit c9152b1

Please sign in to comment.