Consolidate checks for record variables

[jayeshkrishna]

There are several ways that we currently determine if a variable has records,

1. var_desc has a "rec_var" member that is set to 0/1 based on whether the variable has an unlimited dimension
2. var_desc has a "record" member that contains the record being written out (Checking whether the "record" member >= 0 allows us to check if the variable has any records). The "record" member is set when the user calls pio_setframe() on the variable to set its current record. All variables with no record dimensions are supposed to have the "record" member set to -1.
3. comparing number of dimensions in file (fndims) and the number of dimensions in a decomposition (ndims). If fndims is greater than ndims the variable has a record dimension. This is a faulty check and is being fixed now - see PR Support decompositions with extra outermost dimensions #113

The issue with method 2, is that if the user chooses to call pio_setframe() on 1D variables with no record dimensions with a frame number >=0 we no longer can use the "record >= 0" check to determine if the variable has any records. Some E3SM code exhibits this behaviour. This is mostly a bug that we need to fix in the user code. However due to these kinds of bugs we now have code that check for both the number of dimensions and the record member to determine if a variables has any record dims.
The method (3) is being removed (PR #113) as explained above.
Using an approach like (1) should eliminate any need for guessing whether a variable has any records and we can consolidate all logic required to determine whether the variable has any records in a single place/function.

[dqwu]

@jayeshkrishna
In current code, the flag in var_desc is called "rec_var", not "is_rec"

One issue with method 1 is that some E3SM tests do use netcdf variables with limited time dimension. These "quasi-record" variables do not have unlimited dimensions so rec_var flag is set to 0, and we cannot use method1 to identify them. So far, they can only be identified with method 2

Refer to inputdata/ice/dice7/SSMI/ssmi_ifrac.clim.x0.5.090319.nc for an example of this kind of variables:

netcdf ssmi_ifrac.clim.x0.5.090319 {
dimensions:
	time = 365 ;
	nlat = 360 ;
	nlon = 720 ;
variables:
	float ifrac(time, nlat, nlon) ;
		ifrac:long_name = "Sea Ice Concentration" ;
		ifrac:missing_value = 1.e+30f ;
		ifrac:comment = "fractional coverage" ;

[jayeshkrishna]

Thanks, changed "is_rec" to "rec_var" in the description.
The idea is that we should have one way to check if a variable has any record dims and perform record-specific actions based on that information elsewhere in the code. Can't we make this decision based on whether the dims (the outermost dim) are UNLIMITED or not?

[dqwu]

The difficulty is from these record-like (quasi-record) variables and they are very common in E3SM tests. They have limited time dimension but it is fairly OK for users to call pio_setframe on them with valid frame numbers.

PS, PR 113 has added some tests for these variables in pio_decomp_extra_dims.F90.in

[dqwu]

PR 113 uses method 2 to identify strict-record (with unlimited time dimension) or quasi-record (with limited time dimension) variables, assuming that users have correctly set frame numbers on them. No issues have been observed from running E3SM and ESMCI/cime tests so far.

Apparently, method 1 cant identify strict-record variables, but it fails to identify quasi-record variables.

[jayeshkrishna]

If the quasi-record variables don't have an UNLIMITED dimension, why do we treat them as record variables (as compared to regular multi-dim variables)?
(If the user code sets the record number for variables with no record dims, either we should return an error or ignore it.)

[dqwu]

The quasi-record variables are actually handled exactly the same as strict-record ones in PIO2 (have been so for a long time for E3SM tests to pass). The only difference is that users can only set frame numbers within a bound on them since the record dimension is not unlimited.

[jayeshkrishna]

Yeah, we need to find a way to consolidate these checks so that the intent is clearer (are we checking for variables with record dims? are we trying to process variables with non-record dims like record variables etc Is setting the record for a non-record variable ok for all cases)
The idea is to make the intent clear and the code easier to read