Skip to content

Move Benchmark Tests to Subdirectory - #7282

Open
danieldouglas92 wants to merge 2 commits into
geodynamics:mainfrom
danieldouglas92:benchmark_tests
Open

Move Benchmark Tests to Subdirectory#7282
danieldouglas92 wants to merge 2 commits into
geodynamics:mainfrom
danieldouglas92:benchmark_tests

Conversation

@danieldouglas92

Copy link
Copy Markdown
Contributor

Move the benchmark tests into the benchmarks subdirectory.

  • I followed the AI Policy:
    • Significant parts of this PR were written by AI (please add a sentence below).
    • AI has not been used in significant ways.

AI identified tests to move into the subdirectory

@danieldouglas92

danieldouglas92 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

I can already see that some of the tests moved here are the same as tests moved in #7281 (such as the bunge_cookbook). In the manual, this is listed as both cookbooks and benchmarks (bunge, so I suppose it's up to personal preference.

My opinion is that a benchmark is a special case of a cookbook (a benchmark is a square as a cookbook is a rectangle kind of thing), and so I would think if a test corresponds to both a cookbook and a benchmark, it should be put in the benchmark subdirectory. I'm of course open to other opinions on this!

@danieldouglas92 danieldouglas92 changed the title Move Benchmark Tests Move Benchmark Tests to Subdirectory Aug 6, 2026
@tjhei

tjhei commented Aug 6, 2026

Copy link
Copy Markdown
Member

and so I would think if a test corresponds to both a cookbook and a benchmark, it should be put in the benchmark subdirectory.

I think an easier way to decide is to look at the directory structure: if it is in ./benchmarks/ it is a benchmark. Otherwise, it goes into cookbooks. Can you verify?

@danieldouglas92

Copy link
Copy Markdown
Contributor Author

@tjhei The .md file is stored in /cookbooks/, so I'm removing it here and adding it back to #7281

@gassmoeller

Copy link
Copy Markdown
Member

I just wanted to comment that #7158 is almost ready to merge and will likely create conflicts with this PR. Since #7158 has been in preparation for a while and touches a lot more tests than this PR I would like to merge it first. Afterwards this needs to be rebased and conflicts resolved (it is just a single line in the tests .prms that is different).

@danieldouglas92

Copy link
Copy Markdown
Contributor Author

@gassmoeller Thanks for the heads up! Lots of conflicts yes 😅 This PR is not pressing though, so if there are any other PR's from the hackathon where merging this PR first would cause a headache this can wait until they are done. I just wanted to get the process started while it was still on my mind.

@danieldouglas92
danieldouglas92 force-pushed the benchmark_tests branch 4 times, most recently from 9f6310d to a53d446 Compare August 7, 2026 18:39
@danieldouglas92

Copy link
Copy Markdown
Contributor Author

@tjhei @gassmoeller The testers should pass now, the includes for some of the .cc and .prm files had to be updated in this PR and in #7281.

@danieldouglas92

Copy link
Copy Markdown
Contributor Author

/rebuild

@gassmoeller gassmoeller left a comment

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.

Thanks for tackling this. I have a few questions about some of the tests. The general direction is right, I just wasnt sure if these particular tests should move into the benchmarks folder. Let me know what you think about them.

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 latent_heat is a cookbook, not a benchmark?

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.

Yes good catch!

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 dont see this one in the list of benchmark folders, but maybe it just has a different name? I cant check the content of the file on the Github website at the moment (too many tests in one folder :-)). Could you check?

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 test is for the geoid-spectral-comparison benchmark directory!

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.

same question as for spectral_comparison. If this is just a different name, but it does test one of the benchmarks in the benchmarks directory, could we change the name to reflect that? Or is it just testing a benchmark that is in one of the subdirectories inside benchmarks/?

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 is also a cookbook! Updated in #7281

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.

this is a cookbook I think.

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.

yes good catch!

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 the whole suite of vof tests are also not part of the benchmarks directory, or did I not see them? volume_of_fluid could also become its own tests category, given how separate it is from the rest of the code base. Maybe leave it in the general directory for now?

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'll create a new sub-directory for all volume of fluid tests in a follow up PR.

@danieldouglas92

Copy link
Copy Markdown
Contributor Author

The benchmarks that do not seem to have an associated test are:

advection
advection_in_annulus
annulus
buiter_et_al_2008
buiter_et_al_2016
Davies
gravity_thick_shell
gravity_thin_shell
polydiapirs
rayleigh_taylor_instability
slab_detachment
viscosity_grooves
yamauchi_takei_2016_anelasticity
zhong_et_al_93

particle distributions and intergrators probably have tests buried in the particles subdirectory, but I haven't looked into these yet.

@danieldouglas92 danieldouglas92 mentioned this pull request Aug 8, 2026
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants