Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

RadhydroPulseGrey causes stochastic test timeouts and merge failure #624

Closed
BenWibking opened this issue May 1, 2024 · 3 comments · Fixed by #625
Closed

RadhydroPulseGrey causes stochastic test timeouts and merge failure #624

BenWibking opened this issue May 1, 2024 · 3 comments · Fixed by #625
Assignees
@chongchonghe
Labels
bug: wrong answer/failure/crash Something isn't working CI

Comments

@BenWibking
Copy link
Collaborator

Describe the bug
Due to performance variability of the virtual machines that run GitHub actions, the RadhydroPulseGrey test stochastically times out when merging PRs. This causes the merge to fail and the PR to be ejected from the merge queue.

The problem parameters need to be modified so it doesn't take as long to run. The test suite already takes about 1hr 45min to run, even when it doesn't time out.

To Reproduce
Steps to reproduce the behavior:

  1. Merge a PR
  2. See error (e.g.: https://github.com/quokka-astro/quokka/actions/runs/8905036143/job/24455126405):
97% tests passed, 1 tests failed out of 34
Errors while running CTest

Total Test time (real) = 3890.12 sec

The following tests FAILED:
	 28 - RadhydroPulseGrey (Timeout)

Additional context
N/A
@BenWibking BenWibking added the bug: wrong answer/failure/crash Something isn't working label May 1, 2024
@BenWibking BenWibking added the CI label May 1, 2024
@BenWibking
Copy link
Collaborator Author

@chongchonghe This is preventing #621 from merging.

@chongchonghe
Copy link
Contributor

OK, I'll take on this.

@chongchonghe chongchonghe mentioned this issue May 2, 2024
Merged
7 tasks
@chongchonghe
Copy link
Contributor

Addressed in #625

github-merge-queue bot pushed a commit that referenced this issue May 2, 2024
@chongchonghe
Changed parameters in pulse_grey test to reduce runtime (#625)
f7d0e90 
### Description

Address issue #624 . I reduced `max_time` of the
test_radhydro_pulse_grey test by ~5. Now the runtime of this test on my
Mac Studio reduced from 28 s to 6 s.

### Related issues

Fixes #624.

### Checklist
_Before this pull request can be reviewed, all of these tasks should be
completed. Denote completed tasks with an `x` inside the square brackets
`[ ]` in the Markdown source below:_
- [x] I have added a description (see above).
- [x] I have added a link to any related issues see (see above).
- [x] I have read the [Contributing
Guide](https://github.com/quokka-astro/quokka/blob/development/CONTRIBUTING.md).
- [x] I have added tests for any new physics that this PR adds to the
code.
- [x] I have tested this PR on my local computer and all tests pass.
- [x] I have manually triggered the GPU tests with the magic comment
`/azp run`.
- [x] I have requested a reviewer for this PR.
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@chongchonghe chongchonghe

Labels
bug: wrong answer/failure/crash Something isn't working CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Changed parameters in pulse_grey test to reduce runtime quokka-astro/quokka
2 participants
@BenWibking @chongchonghe