E3SM-Project / polaris

Testing and analysis for OMEGA, MPAS-Ocean, MALI and MPAS-Seaice
BSD 3-Clause "New" or "Revised" License
6 stars 13 forks source link

Rename `TestCase` --> `Task` #110

Closed xylar closed 1 year ago

xylar commented 1 year ago

To accommodate more generalization of the types of tasks that polaris is planned for, we want a more generic name for the tasks that polaris can run on their own.

This merge renames "test cases" to "tasks" in most cases and "test suites" to just "suites".

We leave "test groups" for now because the follow-on plan is to remove them entirely.

Checklist

Based off of #112

xylar commented 1 year ago

@matthewhoffman and @trhille, I wanted to make you aware of this change, since it will affect you, too, when you move over to Polaris.

I have received complaints and seen confusion from various folks about why we call things "tests" when they aren't necessarily tests at all but tasks or workflows or something like that. The most important example right now are the "global ocean" test cases in Compass but others related to analysis will crop up in Polaris pretty soon, too. I think the same idea applies to your Greenland, Antarctica, Humboldt and Thwaites "test cases" as well -- they are also the canonical ways of producing meshes for production runs so they're not exactly "tests".

It will be a lot easier to change this now than later. As you can see if you look at the files, this already involves 150 files even though we currently only have 17 tests (or rather tasks) in Polaris so far, not the ~400 that are in Compass.

xylar commented 1 year ago

@cbegeman, I'm certainly not expecting a thorough code review here, just some sanity checking at whatever level you can spare the time for. I am currently running all 17 tests on Chrysalis so I should notice if anything is broken. I have built the docs and they looked okay to me but the changes are so extensive that it's nearly impossible that i didn't miss something important. But I figure we can always fix things as we see them.

xylar commented 1 year ago

@cbegeman, I also decided that it makes sense to do this before (rather than after or during) the move to shared steps. I hope you agree that that's the easiest way.

xylar commented 1 year ago

Testing

I ran the pr suite successfully against a baseline with the current main branch.

I ran all 17 test cases on Chrysalis. I am still waiting on the 1 km RPE baroclinic channel test but all other tests passed (except the analysis step of manufactured_solution because the convergence rate is below the tolerance, as we already know).

cbegeman commented 1 year ago

@xylar I'm totally on board with this, and with merging before the other shared steps changes if that seems easiest to you. I gave the non-documentation diffs a quick browse. Would it be helpful for me to do any testing or grep "test" to try to catch anything you've missed? I'm also ok with doing fixup later as we find things we've missed.

xylar commented 1 year ago

Thanks for taking a look, @cbegeman! This is based off of #112, so if you can give that a look, too, that would be great!

I don't think it's worth you spending time on documentation at this point. After #111, it might indeed be helpful to have you search through the code for "test" in places where it no longer applies (it's fine to still call some things tests, of course).

For now, I think I'm mostly looking for reviews to make sure you're conceptually onboard with the changes I'm making and don't see major conceptual mistakes. I'll keep making sure our regression tests work the same as before.

matthewhoffman commented 1 year ago

@xylar , I like this change in nomenclature!

xylar commented 1 year ago

@cbegeman, thanks for having a look. @matthewhoffman, thanks, that's a relief to hear because I didn't want it to become a big thing.

xylar commented 1 year ago

While I realize this may be less than perfect, I'm going to merge and we'll clean up anything that we find going forward in later PRs.