Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Mock PR to test benchmarking #425

Closed
wants to merge 3 commits into from

Conversation

wilfonba
Copy link
Contributor

This is a mock PR that checks the benchmarking PR. It does the following:

  • Adds a 60 second delay to pre_process for the hypo_hll case
  • Adds a print statement to the 5eq_rk3_weno3_hllc case file to cause failure in input generation
  • Calls s_mpi_abort in simulation for the viscous_weno5_sgb_mono case
  • Allows the IBM case to run to completion

@wilfonba wilfonba requested a review from sbryngelson as a code owner May 20, 2024 14:00
@sbryngelson
Copy link
Member

Ok so the code you wrote throws an error and everything fails everywhere, though this prevents any of the benchmarks from running at all, or at least fails basically immediately. So, we aren't really seeing all of the checks you were hoping to check for (I think).

@wilfonba
Copy link
Contributor Author

I would expect all of the test suites to fail on the first bubbles case since I added an MPI_Abort if bubbles = T. I expected the benchmarks to handle this though since it runs the 4 cases independently. Looking at the CI it appears that neither benchmark actually got a node and ran at all. It could be the case that I needed to introduce a seg fault rather than an MPI_Abort, but I'll have to see what happens when the benchmarks actually run to know.

@sbryngelson
Copy link
Member

Please sync this branch with master

@sbryngelson
Copy link
Member

sbryngelson commented May 23, 2024

I'd like to see this check get to the point where it gets to point where it actually prints benchmark results instead of failing during compilation (presumably with the aforementioned N/A's in the appropriate places).

@sbryngelson
Copy link
Member

The only thing I really want to be checked now is if the benchmark cases run on Delta/non-Phoenix computers.

@wilfonba
Copy link
Contributor Author

I'll do a run on delta

@sbryngelson
Copy link
Member

sbryngelson commented May 24, 2024

I just did. Check the issue for the latest at #396

I'm not really sure what's going on.

@sbryngelson
Copy link
Member

this can be closed due to #497 thanks @ChrisZYJ

@sbryngelson sbryngelson closed this Jul 3, 2024
@wilfonba wilfonba deleted the benchmarkTest branch September 10, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Delta GPUs results give NaNs in the viscous sub-grid bubble benchmark case whereas Phoenix GPUs do not
3 participants