Remove iris.experimental.fieldsfiles module

[lbdreyer]

Remove deprecated module iris.experimental.fieldsfiles which has been replaced

Side note: I thought that if something was experimental it didn't need to be deprecated? Was this a special case because we knew a lot of people used it and want to be clear that they should replace it with iris.fileformats.um.structured_um_loading?

[hdyson]

We have a use case that wasn't catered for with the structured loading, described here: #2083 (comment)

Does structured loading now allow a callback call per field, or is it still a single callback call after all the fields have loaded? To be clear, I'm not saying "don't remove the unstructured loading", I'm just aiming to understand for our own planning whether this change will require us to adapt or not.

[pp-mo]

a special case because we knew a lot of people used it and want to be clear that they should replace it

In my view, yes (I think I probably did it ?)
I think with 'experimental' we will still aim to make people's experience of change easier -- we just don't guarantee anything.

[pp-mo]

@hdyson Does structured loading now allow a callback call per field, or is it still a single callback call after all the fields have loaded?

Structured loading does not implement per-field callbacks in the usual way, because a 'normal' Iris callback is called with a field and the corresponding "raw cube", and the whole point of structured loading is not to make a raw cube for each input field.

In your linked issue, it is never actually explained exactly what you want to do with each field ?
However, if your callback operation can be done on just fields, without reference to a "raw cube", then it is possible to apply structured loading to a file while also modifying or filtering the fields.
In principle, that is...
...The catch is that all of the necessary code components are in iris.fileformats.um._fast_load_structured_fields, which is all private + not public API.
The main reason we kept this private for the time being is that when we made structured loading generally available [[it is needed for "StaGE", for those who know]], we employed some nasty temporary cludges for features that we didn't have time to implement fully !

We could reasonably work on this to make it more flexible, as we did for other file formats when we added iris.fileformats.pp.load_pairs_from_fields, and the like.

[pp-mo]

nasty temporary cludges

For reference,

• the way we (currently) manage pseudo-levels, which is not fully correct : see Structured load pseudolevels #2274
• other outstanding problems are covered by 'fail case tests' in iris.tests.integration.fast_load.test_fast_load

[hdyson]

@pp-mo There's an uncanny coincidence in your previous comments:

In the linked issue it is never actually explained exactly what you want to do with each field ?

the way we (currently) manage pseudo-levels, which is not fully correct : see #2274

Yes, our use case is pseudo levels. We've opted for the approach of keeping the order of the pseudo levels in the source file intact in the output file. We use a callback to add a "pseudo level order" coordinate to each field which we use to ensure the same order is used on save (at the same time, we remove the additional coordinate at save). The devil is in the detail here though: we need to keep track of the pseudo levels we've seen to add the correct value of the "pseudo level order" coordinate to each field.

This can't be done after all the fields are loaded, since the information we need - the order of the fields as they are being read in - is lost by the time a single callback is run.

It's possible our fix is obsolete now. I hadn't seen #2274 before, and our fix for this issue pre-dates it by about 6 months. I'll test out a more recent iris without our pseudo level fix and see if we can remove our callback completely.

[pp-mo]

our use case is pseudo levels ...

For completeness, discussed offline yesterday.

It's possible our fix is obsolete now

I think the existing callback approach will still be be needed, as the the key problem is not with structured load per se : Structured-load itself does preserve file order, dividing the input into adjacent groups with the same stash, lbproc and pseudo-level : it is the PP rules and merge operation which make pseudo-level into a dimension, so then the values get sorted
-- but of course, it is the lack of a callback in structured loads that makes this into a problem.

@marqh pointed out that we can potentially change the PP rules (?selectively) in some way, to retain or record the original order of pseudo-levels.

Another possible temporary fix could be to use iris.load_raw which should preserve order -- and will then allow use of structured loading.
I think that should work at least at present, because our "temporary fix" to support pseudo-levels in structured loading outputs different pseudo-levels in separate result collations, whereas a "correct" solution would produce a single collation with a pseudo-level dimension.
However, I also believe that even a "correct" solution would not impose sorting, so it would still work.
The enemy here, I believe, is the merge process, which makes 'pseudo_level' into a DimCoord, whereas I think a "correct" structured result with unordered vector dimension would result in an anonymous dimension and assoicated pseudo-level aux-coord.
But, I'm not sure of my facts on this : it needs investigating.

Meanwhile, on a broader view, I think it "ought" to be possible to support field-level callbacks within structured loading, as the code structure does allow it : structured-load works from an iterable of fields, so you can potentially get field-level access that way.
The problem is that none of the underlying mechanism is public, which IIRC/IMHO is just because we're still not comfortable that it has reached a stable state... almost like it is still experimental !!
The privacy blocks obvious ways of providing a public feature extension : until we are ready to go public with a low-level API, we can't easily provide a low-level extension, such as the grib/pp 'load_pairs_from_fields' interface.

[pp-mo]

Confirmed offline :
@hdyson says his group have no remaining concern about removal of iris.experimental.fieldsfile

[pp-mo]

We seem to have missed a trick here.
This function still exists, in a slightly enhanced form, as iris.fileformats.um._fast_load.convert_collation
So this could usefully be re-purposed to that, rather than discarded
I.E. --> iris.tests.unit.fileformats.um.fast_load.test__convert_collation.py

I checked + it did work, unmodified : just change the import "... as convert_collation".

[pp-mo]

I think it could be worthwhile to retain this as a totally-simple integration test.

• change from iris.experimental.fieldsfile import load --> from iris.fileformats.um import load_cubes as load
• and then rename --> iris.tests.integration.um.test_ff_load or somesuch ?

[DPeterK]

@lbdreyer this looks fine but you've unaddressed comments from @pp-mo. I agree with @pp-mo here - it would be good to maintain/repurpose the tests that are removed here.

[corinnebosley]

@dkillick Pending passing tests, I am happy with this one.

[corinnebosley]

@dkillick Some licence headers need updating, and the little test changes are playing up.

[corinnebosley]

@dkillick You'll be pleased to hear that the cml chaos appears to be spreading.

[DPeterK]

be pleased to hear that the cml chaos appears to be spreading

Won't I just 😒 Maybe now is the time to just remove cml tests entirely.

[DPeterK]

Oh hangabout, the cml failures here are actually expected - the removed functionality and the replaced functionality do not have quite the same return type, but I didn't get around to fixing yesterday...

[corinnebosley]

Well that's positive. Would still be nice to just get rid of cml tests though.

[corinnebosley]

@dkillick So close...

[DPeterK]

@corinnebosley and a merge conflict 😱

[DPeterK]

So close...

Seriously, the difference is outside of the printed precision 😠 I'll change the CML test type.

[DPeterK]

Just triggerin' the tests...

[DPeterK]

I've updated the result files for the outstanding failing tests. The loader that produces that cube to be compared has changed and does not quite have equivalent functionality; specifically the replacement loader produces a list of cubes while the replaced loader produced a single cube. Somehow merging to create a single cube produces one dimCoord in the result instead of an auxCoord. I don't see this as a problem with the change I've made so I've updated the result file.