Skip to content

gh-1131: Attempt to make reg tests stable#1132

Merged
paddyroddy merged 12 commits into
mainfrom
connor/1131-make-reg-tests-stable
Jun 11, 2026
Merged

gh-1131: Attempt to make reg tests stable#1132
paddyroddy merged 12 commits into
mainfrom
connor/1131-make-reg-tests-stable

Conversation

@connoraird

@connoraird connoraird commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Description

  • Marks some tests as unstable
  • Increases the number of achieved iteration in test_generate[_grf] tests
  • Forces reg tests to be triggered if the tests themselves are updated

Closes: #1131

Checks

  • Is your code passing linting?
  • Is your code passing tests?
  • Have you added additional tests (if required)?
  • Have you modified/extended the documentation (if required)?
  • Have you added a one-liner changelog entry above (if required)?

@connoraird connoraird self-assigned this Jun 10, 2026
@connoraird connoraird added the benchmarks Benchmarking work label Jun 10, 2026
@connoraird connoraird requested a review from paddyroddy June 11, 2026 09:33
@paddyroddy paddyroddy added the bug Something isn't working label Jun 11, 2026
Comment on lines -127 to +128
gls: AngularPowerSpectra = [xpb.ones(10) for _ in range(nth_triangular_number)]
nside = 16
gls: AngularPowerSpectra = [xpb.zeros(10) for _ in range(nth_triangular_number)]
nside = 32

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why changing from ones to zeros?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This prevents the exception "covariance matrix is not positive definite" from being raised, thus allowing us to consume the full generator.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why weren't we doing that before?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, I guess it depends if passing all zeros simulates a real situation worth benchmarking?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It doesn't sound obvious. How long does it take? Should we consult Nicolas on this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably. @ntessore. Does making gls all zeros make this benchmark not realistic?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No, it will produce Gaussian random draws which are all zero, but the amount of work done is still the same.

Another option is

[xpb.ones(10) if i == j else xpb.zeros(10) for i in range(10) for j in range(i, -1, -1)]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, So then I guess we are okay to merge? (assuming an approving review)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I'd prefer Nicolas' suggestion as it reflects a real world case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've updated to use the new example

@connoraird connoraird requested a review from paddyroddy June 11, 2026 10:50
@paddyroddy paddyroddy merged commit 9d22016 into main Jun 11, 2026
13 checks passed
@paddyroddy paddyroddy deleted the connor/1131-make-reg-tests-stable branch June 11, 2026 17:00
@paddyroddy

Copy link
Copy Markdown
Member

Still got

test_generate[numpy-1] (0001_6d0144c) - Field 'mean' has failed PercentageRegressionCheck: 7.222508142 > 5.000000000

in #1133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmarks Benchmarking work bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression tests are flaky again

3 participants