Proposed changes to ESMPy version check errors and warnings

[billsacks]

Requirements

• Reviewed ESMPy Documentation
• Searched GitHub Discussions

Affiliation(s)

NSF-NCAR

ESMF Version

v8.9.0b10-20-gf25e3d4973

Issue

(For the ESMF core team - @anntsay @theurich @danrosen25 @oehmke @uturuncoglu - this is a follow-up from today's discussion. I'd like some feedback on my proposal below - especially see the remarks in bold.)

The ESMPy version check compares the ESMPy package version (esmpyversion, obtained at the time of building/installing ESMPy) against the ESMF library version (esmfversion, obtained from ESMF_VERSION_STRING in the esmf.mk file)).

The current behavior of the ESMPy version check is the following (based on my read of the code, not yet extensive testing, so I may be getting something wrong):

1. If esmpyversion is identical to esmfversion, do nothing
2. If the first two digits of the esmpyversion differ from the first two digits of the esmfversion, raise a VersionMismatch exception. Note: the behavior in the code differs from the comment: the comment says that it's also including the patch version in the comparison, but in fact it is not. (Note that, in python slices, the lower bound is inclusive but the upper bound is exclusive. It can be easy to forget this sometimes-unintuitive rule, so I could imagine that the intent of this code was to do as the comment said rather than as the code actually dictates).

   esmf/src/addon/esmpy/src/esmpy/interface/loadESMF.py

   Lines 87 to 88 in f25e3d4

   	# check if major, minor and patch version numbers are equivalent
   	if esmfvs[0:2] != esmpyvs[0:2]:

3. In any other case, issue a warning. Because of the way beta versions are given, this means that using a beta version will always lead to a warning, even if the esmf and esmpy beta versions are the same (because the version string is modified for the python package). Other examples of situations that will lead to warnings are: esmfversion is 8.8.0 while esmpyversion is an 8.8.0 beta tag (or vice versa); esmfversion is 8.8.0 while esmpyversion is 8.8.1 (or vice versa).

Based on discussion at the ESMF core team meeting, I plan to change this behavior somewhat:

1. As before: If esmpyversion is identical to esmfversion, do nothing
2. As before: If the first two digits of the version differ, raise an exception. This keeps the current behavior of the code rather than the behavior of the comment. It feels like a warning rather than error is appropriate if only the patch version differs but I am open to opinions/feedback here.
3. New behavior: If one is a beta version and the other is a release version, raise an error. (Previously this was just a warning. This change was suggested at the core team meeting.)
4. In any other case, issue a warning. However, I'll try to distinguish in the warning message whether this is due to using different release patch versions (meaning a true version difference) or using a beta version (meaning we cannot fully verify compatibility: the versions might be identical or might not be).

Autotag

@billsacks

[theurich]

@billsacks thanks for putting this together so precisely and understandably!
I agree with everything you propose, except maybe the patch issue question. I am currently leaning toward having this change to what the comment indicates: error out if there is any mismatch in the version triplet, including the patch number. My reasoning is that something was certainly fixed (i.e. has changed) with a patch. It seems confusing at best to allow a patch version mismatch between ESMF and ESMPy. It seems that the recommended situation would always be to have matching version triplets, and so we should strictly checking for that and not support mismatches. I think that the whole beta version mismatch is also just allowed because we cannot test for it any better. If we could, we would/should probably also require exact matches there.

[billsacks]

Thanks for your thoughts, @theurich - I'm happy to go with requiring a match in the full version triplet. I had been concerned that it might cause unnecessary problems for people, but I haven't thought of a specific scenario where this would be likely to come up, and checking the full version triplet does seem cleaner. So I'm happy to move forward with that and then reconsider if people complain.

[billsacks]

I have made the changes discussed here: c272809