CTSM icon indicating copy to clipboard operation
CTSM copied to clipboard

Add CIME unit tests for behavior relied on in CTSM

Open samsrabin opened this issue 1 year ago • 12 comments

From this discussion from today's CTSM SE meeting:

We need to add unit tests to ensure that CIME behaves as we expect it to. As a starting point, we can copy any existing unit tests from CIME to our own repo.

samsrabin avatar Mar 28 '24 15:03 samsrabin

I strongly disagree. CIME unit tests should only be added to cime, not to ctsm. If you want a certain behavior from cime then you need to make that clear in the cime development repository.

jedwards4b avatar Mar 28 '24 15:03 jedwards4b

We can definitely contribute at least some of our tests back to CIME, good idea. And I appreciate the offer to work with us to make sure our uses are supported!

My concern is that CIME may have very good reasons for changing things in ways that break our usage, and I'm not sure how we could guarantee this to be communicated back from CIME to the CTSM team. That would be a pretty big burden on the CIME team! If we have our own tests, we will be able to detect such changes without needing the CIME team to coordinate with us.

samsrabin avatar Mar 28 '24 16:03 samsrabin

That seems backwards to me. Suppose I make a change in cime that breaks a unit test that I do not have access to. What is your proposed workflow to deal with that case?

jedwards4b avatar Mar 28 '24 16:03 jedwards4b

In that case we would figure out what the breaking change was and rewrite our CIME-dependent code around the change (and of course our tests).

samsrabin avatar Mar 28 '24 16:03 samsrabin

Maybe backing up a bit (and apologies if this is blunt). CTSM updated externals in dev172, which broke a number of model behaviors that we were not aware of until later. I don't feel like CTSM SE's should keep track of all the cime changes that came in, but we do need a way to test (or at least know) when behavior breaks. What's the best way to accomplish this, @jedwards4b?

wwieder avatar Mar 28 '24 16:03 wwieder

Okay I think that I understand and perhaps you can rename this issue. If you expect a certain behavior from cime that can be tested within the cime framework that test should definitely be done within cime. If you expect a behavior that can only be tested with ctsm specific tools like run_neon then that test belongs in ctsm and is a run_neon unit test not a cime unit test. Does that make sense to you?

jedwards4b avatar Mar 28 '24 16:03 jedwards4b

I think that makes sense. My understanding of @samsrabin's question was basically, can we use existing CIME unit tests to CTSM to get started developing the unit tests we ultimately need to have in CTSM?

wwieder avatar Mar 28 '24 16:03 wwieder

@jedwards4b I think that's not quite right. What we were thinking was that it would be useful for us to have our own CIME-specific tests in the same way that it's useful for a developer to test against, say, the Facebook API. Some random developer wouldn't expect Facebook to coordinate with them for API updates. The difference here, of course, is that y'all aren't Facebook! So if you're willing to bring in new CIME unit tests that we write to check behaviors we rely on, that would be much appreciated.

samsrabin avatar Mar 28 '24 16:03 samsrabin

@jedwards4b yes that's the way I was thinking about this as well. We might add some unit tests to cime for behavior we rely on in CTSM.

We also plan on adding unit tests to CTSM that cover the behavior we expect in CIME. And those unit tests will trigger when there is a behavior change in CIME when we update externals.

So that's the two things you talked about above.

In addition to that we plan on adding some prealpha tests in CTSM that will detect problems in external updates in the CESM workflow. That will give us a heads up about particular cime updates that break our expected behavior.

The third thing we are trying to figure out though -- is a way to figure out changes in CIME behavior we rely on when externals are updated. That's where one idea is to copy the unit tests from cime which will correspond to a given version of cime. If those tests break with a cime update for CTSM -- we'll investigate the differences to the CIME unit tests which will tell us what behavior changed in CIME, and how we need to compensate in CTSM.

I think there might be other things to try in this last area.

  • Just doing a diff between the CIME unit tests to get an idea of the behavior changes
  • Possibly doing git bisect on cime versions to see when the behavior changed
  • Going through git logs about the changes in cime.
  • If there is specific things we could look for in the cime git logs that would talk about behavior changes that could help...

This last part we are just brainstorming on what to do...

ekluzek avatar Mar 28 '24 16:03 ekluzek

Yes, as @samsrabin says, we potentially want to add some CIME unit tests around expected behavior. Which is exactly what you said above

"If you expect a certain behavior from cime that can be tested within the cime framework that test should definitely be done within cime."

Part of what @samsrabin is asking for is permission to do that. And it sounds like you agree with that idea.

ekluzek avatar Mar 28 '24 16:03 ekluzek

We'll add an issue to CIME about adding tests to CIME when we actually have a clearer picture of what this will look like on the cime side.

ekluzek avatar Mar 28 '24 16:03 ekluzek

Thanks for that explanation, @ekluzek! To expand a bit:

The third thing we are trying to figure out though -- is a way to figure out changes in CIME behavior we rely on when externals are updated. That's where one idea is to copy the unit tests from cime which will correspond to a given version of cime. If those tests break with a cime update for CTSM -- we'll investigate the differences to the CIME unit tests which will tell us what behavior changed in CIME, and how we need to compensate in CTSM.

This is what I was thinking to do as a solution to my concern of "I'm not sure how we could guarantee this to be communicated back from CIME to the CTSM team."

samsrabin avatar Mar 28 '24 16:03 samsrabin