Reference Issues/PRs

Partially addresses #22827

Task List

• address FIXMEs for sklearn version 1.2 in code (currently at 1.2.dev0) --> out of scope for this PR
• test_linear_regression_sample_weights
• test_raises_value_error_if_sample_weights_greater_than_1d (--> no seed change; this tests ValueError)
• test_linear_regression_sparse
• test_linear_regression_sparse_equal_dense (--> out of scope, will be deprecated in v1.2)
• test_linear_regression_multiple_outcome (--> no seed change, random data has no impact on test)
• test_linear_regression_sparse_multiple_outcome
• test_linear_regression_positive_vs_nonpositive
• test_linear_regression_positive_vs_nonpositive_when_positive
• test_linear_regression_pd_sparse_dataframe_warning (--> no seed change; this asserts warnings)
• test_preprocess_data
• test_preprocess_data_multioutput
• test_preprocess_data_weighted
• test_sparse_preprocess_data_offsets
• test_dtype_preprocess_data
• test_rescale_data_dense
• todo: check remaining tests in test_base.py

What does this implement/fix? Explain your changes.

• Added global_random_seed to all seeded tests in sklearn/linear_model/tests/test_base.py
• The only test that failed on a few seeds was test_linear_regression_sample_weights
  • Fixed by removing the arbitrary check assert reg.score(X, y) > 0.5 - scores can be below 0.5 for some cases of fit_intercept=False
  • Now test passes for all seeds, more info see below

Long version:

Observations:

• for specific seeds, i.e. seeds (3, 15, 17, 20, 23, 25, 54, 64, 79, 91, 99), the test test_linear_regression_sample_weights fails if fit_intercept=False. The test passes if fit_intercept=True:

  >       assert reg.score(X, y) > 0.5
  E       assert 0.3016564425292758 > 0.5
  E        +  where 0.3016564425292758 = <bound method RegressorMixin.score of LinearRegression(fit_intercept=False)>(array([[-0.60612102, -1.05993975, -0.55091967, -0.27568627,  1.22225373],\n       [-0.90585899,  0.06935217,  2.1786556...215,  2.11737829, -0.44639435, -0.66693906],\n       [ 0.48079725,  1.98498011,  0.39190894,  2.3982028 ,  2.41736027]]), array([-1.61411983, -0.21642658,  1.66363212, -0.12201627,  1.39842066,\n       -0.63836468]))
  E        +    where <bound method RegressorMixin.score of LinearRegression(fit_intercept=False)> = LinearRegression(fit_intercept=False).score
  
  sklearn/linear_model/tests/test_base.py:80: AssertionError
  

  In the failing cases with one of these seeds and fit_intercept=False, reg.score is lower than 0.5. Setting fit_intercept=True fixes this for all seeds.

• Any other combination than n_samples, n_features = 6, 5 results in more failed tests. E.g. reducing to n_samples, n_features = 5, 4 does not help even with fit_intercept=True

• n_samples needs to remain at a low value, because it will be hard to fit a linear regression to more than a few randomly chosen samples/targets. E.g. every test fails with n_samples = 30.

Possible solutions

• remove assert reg.score(X, y) > 0.5

  • we picked that one

• run only tests with working combinations of global_random_seed and fit_intercept; e.g.

  if np.isin(global_random_seed, (3, 15, 17, 20, 23, 25, 54, 64, 79, 91, 99)) and not fit_intercept:
    pytest.skip("Unsupported Configuration")

• mark bad combinations as xfail - we have to introduce a gate-keeper fixture for this:

  @pytest.fixture
  def xfail_selected_intercept_seed_combos(request):
      fit_intercept = request.getfixturevalue("fit_intercept")
      seed = request.getfixturevalue("global_random_seed")
  
      # test is known to fail on these seeds for `fit_intercep=False`
      allowed_failures = ((False, s) for s in (3, 15, 17, 20, 23, 25, 54, 64, 79, 91, 99))
      if (fit_intercept, seed) in allowed_failures:
          request.node.add_marker(
              pytest.mark.xfail(reason="skipping bad combinations of tests", strict=True)
          )
  
  
  @pytest.mark.parametrize("array_constr", [np.array, sparse.csr_matrix])
  @pytest.mark.parametrize("fit_intercept", [True, False])
  @pytest.mark.usefixtures("xfail_selected_intercept_seed_combos")
  def test_linear_regression_sample_weights(
      array_constr, fit_intercept, global_random_seed
  ):

Either one does the job and introduces more seeds to the test. I prefer the latter option though. It tells a better story: it tests every combination and safeguards against silently passing combinations in the future.

Comments and opinions are very much appreciated! :)

Any other comments?

I created a task list to track the implementation progress of global_random_seed. Also, let's discuss if we tackle the two version 1.2 related FIXME items in this or in another (linked?) PR (how is that kept track of normally?)