Provide info on pp-field indices in the file for structured um loads.

[pp-mo]

This intends to address problems discussed in this comment and follow-ons.

Implementation notes :

• The information is attached to the 'field' arg in the load callback, i.e. in this case a FieldCollation.
• The caller should be able to use this to annotate the multidimensional collated cubes as required : I believe this is what the users's existing code does (using normal load to annotate each raw cube with an extra "pseudolevel ordering" coordinate).
• the old 'FieldCollation' class is now renamed 'BasicFieldCollation' : the new, public 'FieldCollation' is a derived class which adds the location-in-input-file info. This preserves the original, cleaner concept.

Todo:

• user to confirm suitablility
• needs a whatsnew

[stickler-ci]

F401 'iris.fileformats.um.structured_um_loading' imported but unused

[stickler-ci]

E501 line too long (87 > 79 characters)

[pp-mo]

Remaining errors now look to be unrelated (= present business-as-usual problems)

[pp-mo]

User confirmed it does work for them.
✔️

[pp-mo]

DO NOT MERGE
Now good to go AFAIK 🤞

[pelson]

What is the usecase for needing the position in the file? Is it so that the order is round-tripped or is there a deeper meaning?

If it is simply for round-tripping, users are going to need to dig deep into the Iris API anyway (to control the save order), and technically the order isn't entirely lost... we store a pointer to the data which is possible to sort to get back to the original order (ironically, this was much easier in biggus, but feasible in dask).

Without looking too much at the implementation detail here, the first things that occur to me are:

• The changes look involved for a use-case that doesn't appear to have been fully clarified. In order to take on such churn, I think we need to know that we are doing it for good reason
• You are touching performance sensitive code. Can you give some reproducible remarks (read: not a full-on performance assessment, more a quick sense of the implications) on what impact the changes have?

[pp-mo]

Is it so that the order is round-tripped

Exactly.
They are processing and re-saving data, and need to preserve the original file order for compatibility with existing applications.
To be exact, the input is fieldsfiles, for which field order is supposed to be defined by format specification, but there is an unfortunate loophole relating to 'pseudo-level' encoding, for which the occurrence order is, effectively, significant but not predictable.
Correct me if wrong @hdyson ?

TBH I didn't consider it necessary to explain the need in detail.
I just thought it was clear there is an awkward problem here you can't get around, and therefore merits fixing.

technically the order isn't entirely lost ... we store a pointer to the data

That is true, but this enables the purpose to be achieved using public supported API instead of relying on hidden implementation detail.

[hdyson]

To be exact, the input is fieldsfiles, for which field order is supposed to be defined by format specification, but there is an unfortunate loophole relating to 'pseudo-level' encoding, for which the occurrence order is, effectively, significant but not predictable.
Correct me if wrong @hdyson ?

Yes, that's correct, with the caveat that this is for ancillary files: other forms of fields files may or may not have the same behaviour.

As for @pelson's concern about digging deep into the iris API to preserve the order on save, we kind of bypass that by not using iris for the save. More accurately, for ancillary file saving, we use iris cube2pp to get a bunch of pp fields, sort them depending on information from the cube, convert them to mule fields, and use mule to save. So for us, it's sufficient that the order of the pseudo levels is preserved from the pp or fields file to the cube.

At present, we use our own callback on loading each pp field to ensure we get a pseudo level coordinate with the points in the same order as the original file, and so can use this information for the sort when saving. Structured loading is going to remove this capability because the callback is only triggered after all fields are loaded and merged, so we've already lost the information we're trying to preserve.

[pp-mo]

Rebased !
Just reviewing this morning, whether this can be resolved easily + signed off.
Is this still of interest @hdyson ?

[hdyson]

Is this still of interest @hdyson ?

It is, yes - we need to be able to preserve the pseudo level order, and would like to be able to use the structured loading, so this looks like it hits the sweet spot of giving us the structured loading benefit alongside retaining the existing functionality.

[pp-mo]

Just to paraphrase + review @pelson previous comments above.
I believe I already addressed most of these, as follows ...

• What is the usecase for needing the position in the file? Is it so that the order is round-tripped or is there a deeper meaning? ... I think we need to know that we are doing it for good reason ...

  • the driving usecase seems specific, but the proposal just restores info available to standard load callbacks but missing in structure loads : i.e. which index in which file each field came from

• The changes look involved for a use-case that doesn't appear to have been fully clarified. In order to take on such churn, I think we need to know that we are doing it for good reason ...

  • I honestly don't see that the changes are very involved. An index property is added to each field; a new collation type stores the filepath, and adds access methods for the new info.

• users are going to need to dig deep into the Iris API anyway (to control the save order) ...

  • not really, you can use the iris.fileformats.pp.save_pairs_from_cube interface effectively to do this.

• technically the order isn't entirely lost... we store a pointer to the data ...

  • this is not available via public API

The remaining point is about performance, which I think is worthy of more comment ...

• You are touching performance sensitive code. Can you give some reproducible remarks (read: not a full-on performance assessment, more a quick sense of the implications) on what impact the changes have?

From a performance PoV, nothing has changed in the structure identification step. iris.fileformats.pp.PPField has gained an extra property assignment in __init__.
Loading (structured loading only) has an additional iterator wrapper, that assigns an index value into each field .
I don't anticipate anything very costly in this.

The purpose of structured um loading is to avoid : (a) creating cube objects for each PPField object, (b) running translation rules on each field, and (c) a raw-cube merging stage.
None of that changes, so the only real cost is a small slowdown in creating PPField objects themselves.
This does apply to 'normal' loads as well. It is in principle avoidable (we didn't need to initialise the private '_index_in_structured_load_file' property), but I think it is wiser to include the new thing in the __init__.

Here are some desktop measures of the timing differences, on my machine, loading from dataZoo/FF/ff_variants/ff_64.ff, containing 1202 fields, and comparing standard Iris 2.1.0 with the branch :

• for making a list of PPFields with fields = list(iris.fileformats.um.um_to_pp(path)) :
  • old Iris 2.1.0 : (0.533359, 0.528687, 0.527758, 0.556547, 0.529979) ~ 0.5353 seconds
  • new branch code : (0.533923, 0.528155, 0.526929, 0.526727, 0.530310) ~ 0.5292 seconds
• a 'normal' load with data = iris.load(path) :
  • old : (5.352467, 5.332399, 5.365070, 5.373689, 5.333997) ~ 5.3515 seconds
  • new : (5.367050, 5.273514, 5.282430, 5.262198, 5.349504) ~ 5.3069 seconds
• a 'structured' load with iris.fileformats.structured_loading and iris.load :
  • old : (2.515778, 2.420102, 2.439936, 2.452089, 2.424549) ~ 2.4505 seconds
  • new : (2.456725, 2.454413, 2.452883, 2.425538, 2.430701) ~ 2.4441 seconds

Not a great testcase, but nothing much to report.

[DPeterK]

@pp-mo looking good so far - I've only made a couple of very minor comments. I haven't yet managed to wrap my head around the new test module because it's quite opaque and it seems that most of the pertinent code is elsewhere!

[DPeterK]

It's a small point perhaps, but it's not guaranteed that the generator will produce BasicFieldCollation objects, given that you can specify the collation_class in the constructor.

[pp-mo]

ok

[DPeterK]

@pp-mo just a couple more comments from me. Nothing particularly of note though!

[DPeterK]

@pp-mo don't forget to also change the module docstring to also point to this new class.

[pp-mo]

ok, good spot !

[pp-mo]

I have also changed the BasicFieldCollation as FieldCollation usage in this file, because I think that is unreasonably confusing.

[DPeterK]

@pp-mo nice work! Thanks for pushing this through.