Added auxiliary coordinates lead to (time) efficiency loss

[BenMGeo]

I recently realized that we experience quite an efficiency loss due to our added coordinates (by the backend) concerning time.

The backend adds the auxiliary coordinates day_of_year, day_of_month, year, month_number. If you leave them in for further calculations based on collapsing along the time axis in diagnostics, this leads to a significant increase of processing time!

Here is a "simple" example that shows the increasing run time:

import iris
import iris.coord_categorisation
from scipy.stats import linregress

import functools
import time

def timer(func):
    # from https://realpython.com/primer-on-python-decorators/#simple-decorators
    @functools.wraps(func)
    def wrapper_timer(*args, **kwargs):
        start_time = time.perf_counter()    
        value = func(*args, **kwargs)
        end_time = time.perf_counter()      
        run_time = end_time - start_time    
        print(f"Finished {func.__name__!r} in {run_time:.4f} secs")
        return value
    return wrapper_timer


filename = iris.sample_data_path('E1_north_america.nc')
air_temp = iris.load_cube(filename, 'air_temperature')

# removing extra coordinates
air_temp_small = air_temp.copy()

air_temp_small.remove_coord("forecast_reference_time")
air_temp_small.remove_coord("forecast_period")

# adding extra coordinates
air_temp_big = air_temp.copy()

iris.coord_categorisation.add_month_number(air_temp_big,"time","month")
iris.coord_categorisation.add_day_of_year(air_temp_big,"time","doy")

@timer
def Mean_Cube(cube, coords):
    # replacement for a more complex function
    return cube.collapsed(coords, iris.analysis.MEAN)

small_time = Mean_Cube(air_temp_small, "time")
# Finished 'Mean_Cube' in 0.0035 secs
print(small_time)
# ...

big_time = Mean_Cube(air_temp_big, "time")
# Finished 'Mean_Cube' in 2.2839 secs
print(big_time)
# ...

The processing time increases by a factor of ~1000!!!

print(big_time.data - small_time.data)
# = all zeroes!

The content itself is the same.

This does not occur for collapsing along dimensions without auxiliary coordinates.

small_lat = Mean_Cube(air_temp_small, "latitude")
# Finished 'Mean_Cube' in 0.0046 secs
print(small_lat)
# ...

big_lat = Mean_Cube(air_temp_big, "latitude")
# Finished 'Mean_Cube' in 0.0052 secs
print(big_lat)
# ...

This needs to be told to the diagnostic developers if they use iris in their diagnostics (unknown if this is relevant for other packages besides iris). I'm not sure if this is relevant for the backend, though.

I will address this issue also to the iris community, as I think this is a solvable performance issue. (as discussed with @bjoernbroetz)

[mattiarighi]

Transferred to core since it affects the preprocessor.

[BenMGeo]

Transferred to core since it affects the preprocessor.

But do not forget to add any relevant outcome to the documentation (therefore, I raised the issue in the Main Tool repository).

[BenMGeo]

The accompanying issue at iris (SciTools/iris#3363) @bjlittle

[valeriupredoi]

nice one @BenMGeo 🍺

[valeriupredoi]

also +1 for using decorators 😁 Will you be in touch with the iris guys find out when the patch will go in, I suppose iris 2.3

[BenMGeo]

I don't know if we should/can push (like in "make it urgent") the pull request over at iris.
I'm not sure if I have the capacity to test the changes rcomer implemented.
I also assume that the changes will be in a soonish release as tests are implemented and they realized that it is a severe issue.

[BenMGeo]

It seems to be included in 2.3 as the pull request was merged for this milestone:
https://github.com/SciTools/iris/milestone/40

[bouweandela]

The backend adds the auxiliary coordinates day_of_year, day_of_month, year, month_number.

Maybe we could move this into a separate preprocessor function, for those users who need it? I don't think this is part of CMOR or cf-conventions, so I'm not sure why we're doing this anyway. @jvegasbsc Can you comment on this?

[jvegreg]

We decided to add them because they are useful at several preprocessor functions and adding them at this step was thought to be better than dealing with them at each point they are used.

It is not a problem at all to remove that part of the code and change the functions to add them if needed

[bouweandela]

It is not a problem at all to remove that part of the code and change the functions to add them if needed

I think that might be better, it makes the code easier to understand if all code that does one thing is in one place. Also, we already have a preprocessor function that needs to remove these extra coordinates before it can do it's job:

ESMValCore/esmvalcore/preprocessor/_time.py

Lines 316 to 320 in 08eb1e4

	# remove aux coords that will differ
	reset_aux = ['day_of_month', 'day_of_year']
	for auxcoord in cube.aux_coords:
	if auxcoord.long_name in reset_aux:
	cube.remove_coord(auxcoord)

We could add a new preprocessor function to add any of the extra coordinates provided by iris.coord_categorisation.add_* if diagnostics need them, preferably quite late in the preprocessing chain to minimize the risk of accidentally deleting those coordinates again.

[BenMGeo]

Just make sure that the diagnostic editors know about that change. Most of my code currently assumes that these coordinates are there, as I set them as a standard. I will happily change that, but others might run into issues if they are not aware.