singleton and generic level

[doutriaux1]

Koji OGOCHI ogochi@jamstec.go.jp
Japan Agency for Marine-Earth Science and Technology (JAMSTEC)

reports:

I found a serious problem that appears if a variable
has both generic_level and singleton in its dimensions.
The variable ec550aer is the case.

I fixed this problem like the attached patch.

Regards,
Koji Ogochi

Patch attached.

cmor_variable.patch.txt

[taylor13]

This variable is in CMOR tables "Emon" and "6hrLev" where in Emon, for example, ec550aer has coordinates:
longitude latitude alevel time lambda550nm
(and lambda550nm is a singleton coordinate).

[martinjuckes]

Just a comment: ec550aer (Emon and 6hrLev) appear to be the only variables which have both a vertical dimension and singleton coordinate variable. This coordinate variable is a wavelength, so it should be acceptable as far as CF is concerned.

[taylor13]

Yes, I think the variable is defined correctly and consistent with the CF conventions. We'll need to check that CMOR, with the patch, handles everything correctly.

[taylor13]

See also PCMDI/cmip6-cmor-tables#230

[wachsylon]

I get the same Error for rsdcsafbnd in E3hrPt.

[taylor13]

It's not clear that the two errors are really the same. rsdcsafbnd in E3hrPt is a function of 5 coordinates: longitude latitude alevhalf spectband time1. Have we tested whether CMOR correctly deals with 5? Can we generate a test case?

Regarding ec550aer, it is also a function of 5 coordinates, but the last one is a scalar dimension (not a true coordinate), so it gets handled differently by CMOR. This would require a different test code.

We should try to fix these bugs as soon as we can.

[taylor13]

Looking at the patch, I'm concerned that it might break something else (not tested by our test codes). Do we understand what Ogochi has done?

[mauzey1]

@taylor13
The changes in Ogochi's patch are fixing a bug in cmor_variable() where it's getting the 'Z axis' dimension for the list of axes in a variable.

cmor/Src/cmor_variables.c

Lines 1717 to 1777 in 9238f78

	k = 0;
	for (i = 0; i < lndims; i++) {
	char *name = refvar.dimensions[i] >= 0
	? cmor_tables[refvar.table_id].axes[refvar.dimensions[i]].id
	: "";
	if ((strcmp(name, "latitude") == 0 || strcmp(name, "longitude") == 0)
	&& grid_id != 1000) {
	/* -------------------------------------------------------------------- */
	/* ok we are dealing with a "grid" type of data */
	/* -------------------------------------------------------------------- */
	if (did_grid_reorder != 0)
	continue;
	for (j = 0; j < cmor_grids[grid_id].ndims; j++) {
	cmor_vars[vrid].axes_ids[k] = cmor_grids[grid_id].axes_ids[j];
	k++;
	}
	did_grid_reorder = 1;
	} else if ((refvar.dimensions[i] == -2)
	|| (cmor_tables[CMOR_TABLE].axes[refvar.dimensions[i]].value
	== 1.e20)) {
	/* -------------------------------------------------------------------- */
	/* not a singleton dim */
	/* -------------------------------------------------------------------- */
	iref = -1;
	for (j = 0; j < lndims; j++) {
	if ((refvar.table_id == cmor_axes[laxes_ids[j]].ref_table_id)
	&& (refvar.dimensions[i]
	== cmor_axes[laxes_ids[j]].ref_axis_id)) {
	cmor_vars[vrid].axes_ids[k] = laxes_ids[j];
	}
	/* -------------------------------------------------------------------- */
	/* -2 means it is a zaxis */
	/* -------------------------------------------------------------------- */
	if (refvar.dimensions[i] == -2) {
	if (cmor_axes[laxes_ids[j]].axis == 'Z')
	cmor_vars[vrid].axes_ids[k] = laxes_ids[j];
	}
	}
	k++;
	} else if (refvar.dimensions[i] == -CMOR_MAX_GRIDS) {
	/* -------------------------------------------------------------------- */
	/* ok this is either a lat/lon */
	/* -------------------------------------------------------------------- */
	for (j = 0; j < ndims; j++)
	if (axes_ids[j] < -CMOR_MAX_GRIDS + 1)
	break;
	l = j;
	for (j = 0; j < cmor_grids[grid_id].ndims; j++) {
	cmor_vars[vrid].axes_ids[k] = cmor_grids[grid_id].axes_ids[j];
	k++;
	i++;
	}
	i--; /* one too many i adds */
	}
	}

The above segment of code is meant to get all non-singleton dimensions from laxes_ids into cmor_vars[vrid].axes_id. cmor_vars[vrid].axes_id should be indexed with the variable k, which doesn't get incremented if refvar.dimensions[i] is a singleton dimension. In the segment that found the Z axis, cmor_vars[vrid].axes_ids was originally indexed by i, which would leave a gap in that array when the singleton value is skipped. Ogochi's patch changes the index to k.

The other changes are for detecting when values from refvar.dimensions are negative numbers before using them as indexes to an array. However, there are still parts of cmor_variable() that use those values as array indexes without checking if the indexes are negative.

[wachsylon]

Hi,
I had a similar problem for hus in E3hrPt. I received:

C Traceback:
! In function: cmor_variable
! called from: cmor_zfactor
! 

!!!!!!!!!!!!!!!!!!!!!!!!!
!
! Error: You defined variable 'ps' (table E3hrPt) with axis id 'time1' which is not part of this variable according to your table, it says: ( time latitude longitude )
!
!!!!!!!!!!!!!!!!!!!!!!!!!

There is a definition of ps in CMIP6_E3hrPt.json which has "dimensions": "longitude latitude time1".
However, for the z_factors, CMOR seems to use the dimensions defined in CMIP6_formula_terms.json
I discovered that if I change the dimension time for ps in CMIP6_formula_terms.json to time1, CMOR succeeded.

Best regards,
Fabi

[taylor13]

hus is a function of plev7h, which is a simple pressure coordinate and should not have formula_terms or zfactors.

rsucsaf and rsdcsaf in E3hrPt are functions of longitude latitude alevhalf time1. For these and variables like these, you should define ps1 (not ps) in CMIP6_formula_terms.json when you have, for example, a sigma type coordinate. Note that ps1 is a function of time1 and its outname is ps.

[kjoti]

@taylor13 @mauzey1
I will explain what I did.

Conditions

1. The variable has a generic level, i.e., refvar.dimensions[i] == -2
2. The variable has any singleton dimensions and users omit some axes_ids of them when invoking cmor_variable(), i.e., refvar.ndims > lndims

What's happened in code

Occurred in cmor_variable.c.

At line 1466, the conditional (refvar.ndims + aint != lndims) is true, so the following then-block is executed. In the then-block, the expression cmor_table[].axes[refvar.dimensions[i]].value != 1.e20 (line:1474) does not work correctly when the loop index i is corresponding to the generic level, because refvar.dimensions[i] is -2 and it is unlikely that the value retrieved by invalid memory access is equal to 1.e20. If the value is not equal to 1.e20, it is a singleton dimension. Therefore, the generic level dimension is misinterpreted as a singleton dimension. Finally, at line 1530, cmor_axis is invoked with wrong information and the program crashes.

There is one more bug (line: 1718 & 1746). In this loop, the axes IDs are copied from laxes_ids[j] to cmor_vars[vrid].axes_ids[k] for each i. The index of the destination is k and the index of the source is j. So the index in LHS (line: 1758) should be k not i.

Fixing the above two, code may work even though out-of-range errors remain yet. In my debug execution, cmor_tables[0].axes[-2].id was \x00\x00..., so the string comparison with strcmp worked by accident.