Extend ACCESS-OM3 filename regex

[dougiesquire]

Closes #176. Decided just to add a quick fix for this. However, this whole process needs to be revisited soon as it clearly isn't scaling to the range of ACCESS outputs.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.33%. Comparing base (6be33d8) to head (06c298e).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
+ Coverage   96.98%   97.33%   +0.35%     
==========================================
  Files           9        9              
  Lines         631      902     +271     
==========================================
+ Hits          612      878     +266     
- Misses         19       24       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[anton-seaice]

I think this won't work if the filename patterns change during the experiment? e.g. half the results use ...1900-XX-XX.nc and the other half use 1900-01-daily.nc ?

[anton-seaice]

We should also add "access-om3.cice.h.daily.1900-01" ?

[dougiesquire]

I think this won't work if the filename patterns change during the experiment? e.g. half the results use ...1900-XX-XX.nc and the other half use 1900-01-daily.nc ?

I'm not sure what you mean by "won't work" here. They would get distinct fileids and so be considered distinct datasets, which I think is the correct behaviour for the way things are set up currently.

[dougiesquire]

I think we're now heading (back) towards using CMIP6-like vocab for the frequency

E.g. access-om3.cice.h.1day.mean.1900-01.nc

So let's put a pin in the PR until the naming is resolved.

[dougiesquire]

Okay, I think we've finialised a format - see here. I'll update this PR.

[anton-seaice]

I'm not sure what you mean by "won't work" here. They would get distinct fileids and so be considered distinct datasets, which I think is the correct behaviour for the way things are set up currently.

My concern is that the archive script which turns "access-om3.cice.h.1900-01-01" into "access-om3.cice.h.1900-01-daily" might fail, and then get fixed. Meaning the catalog ends up with two fileids when ideally we want them to be considered the same dataset.

[dougiesquire]

My concern is that the archive script which turns "access-om3.cice.h.1900-01-01" into "access-om3.cice.h.1900-01-daily" might fail, and then get fixed. Meaning the catalog ends up with two fileids when ideally we want them to be considered the same dataset.

Right, this sort of issue can't really be handled with the way dataset grouping is done at the moment. The fix for this currently would be to go back and concatenate the files that were missed. I think in your specific example that's actually the best solution anyway. But we will be revisiting how the grouping is done soon to make it more robust.

[anton-seaice]

I think with access-om3.cice.1day.mean.1900-01.nc from COSIMA/access-om3#190 (comment) we might not need to make a change ?

[dougiesquire]

@marc-white I got halfway through this and then got distracted. The issue is that the regex description for ACCESS-OM3 is overly prescriptive and doesn't work with, for example, annual files. Would you be able to take this over? (It may be easier to just close this and start again, given your recent changes)

This comment contains a summary of what ACCESS-OM3 output should look like in the not-too-distant future. @anton-seaice can also help answer any questions.

[marc-white]

Yep, happy to take over.

[minghangli-uni]

Hi @marc-white, I wanted to check in regarding the status of this PR. I am currently having issues with the regex description for ACCESS-OM3, as it doesn’t seem to work with annual files.

[marc-white]

@minghangli-uni this got buried under other things, but I'll take a look now.

[marc-white]

So is this where the filename formats got to? The original issue is pretty convoluted... COSIMA/access-om3#190 (comment)

[marc-white]

@minghangli-uni could you give me some sample filenames that you're trying to work with?

[minghangli-uni]

Sure, you can find some in /scratch/tm70/ml0072/access-om3/archive/longerexpt5_rr_mean_snap3-perturb-5873b4db/output000

[marc-white]

@minghangli-uni I'm a bit confused, which are the annual files in there you're having issues with? I can only see what appear to be monthly files, with a smattering of others...

[minghangli-uni]

I think the problem I am having is related to the annual filenames. here

    PATTERNS = [
        rf"[^\.]*\.{PATTERNS_HELPERS['om3_components']}\..*({PATTERNS_HELPERS['ymds']}|{PATTERNS_HELPERS['ymd']}|{PATTERNS_HELPERS['ym']})$",  # ACCESS-OM3
    ]

It now doesnt support something like ['y']?

[marc-white]

@minghangli-uni ah yes, I see now. @dougiesquire has already made that update in this pull request, although I can see a couple of other things missing when I compare to your sample directory that I'll need to update.

[marc-white]

@minghangli-uni @dougiesquire @anton-seaice is the GMOM_JRA filename pattern something we need to keep supporting, or should everything under ACCESS-OM3 be changed? I saw @dougiesquire had changed all the filename parsing tests away from GMOM_JRA, but I'll also need to update some test data within the package if it's really going away.

[anton-seaice]

All of our current configs use the names 'access-om3' but I wouldn't rely on that too much. It would be nice to be able to change that name without having to change any code in the catalog builders.

[marc-white]

@anton-seaice it's more that the current regex updates for ACCESS-OM3 have broken one of the GMOM_JRA patterns, so I need to know if I need to rectify that, or if it's OK to dispose of support for that pattern (plus I'll need to grab some new test data for the catalog creation).

[anton-seaice]

Which pattern isn't working ? Its probably fine to dispose of it but also nice if users can make modifications to filenames if they want without breaking things

[marc-white]

I managed to work out the issue with the regular expressions, and it seems to now be in a state where it works with both the new access-om3 and older GMOM_JRA patterns.

During this, I ran into an interesting regex conundrum that sucked up a lot of my day, which I wondered if anyone had seen before...

The current AccessOM3Builder in main uses an OR-style block to match the date group part of a filename:

PATTERNS = [
        rf"[^\.]*\.{PATTERNS_HELPERS['om3_components']}\..*({PATTERNS_HELPERS['ymds']}|{PATTERNS_HELPERS['ymd']}|{PATTERNS_HELPERS['ym']})$",  # ACCESS-OM3
    ]

(look at the group in parenthesis that references the 'ymds', 'ymd', etc. keys of PATTERNS_HELPERS, with | in between each option).

The re documentation says that | searches left to right, taking the first match it finds. So far, so good - we want to see if we can match a Y-M-D-S pattern, if that doesn't work we try the simpler Y-M-D pattern, and so on.

However, if I added a Y only pattern to the end of the | chain, this would happen:

In [42]: b = re.search(AccessOm3Builder.PATTERNS[0], "GMOM_JRA_WD.ww3.hi.1900-01-03-00000.nc")

In [43]: b.group(0)
Out[43]: 'GMOM_JRA_WD.ww3.hi.1900_01_03_00000.nc'

In [44]: b.group(1)
Out[44]: '0000'

It's almost like the | search suddenly decided to deliberately search for the minimum match possible, and gave me back the last year match it found. Removing the Y pattern gets me back to the result I was expecting:

In [32]: b = re.search(AccessOm3Builder.PATTERNS[0], "GMOM_JRA_WD.ww3.hi.1900-01-03-00000.nc")

In [33]: b.group(1)
Out[33]: '1900-01-03-00000'

Any thoughts from anyone (but particularly @dougiesquire )?

[dougiesquire]

Any thoughts from anyone (but particularly @dougiesquire )?

I agree that's odd. Unfortunately nothing springs to mind sorry. I'm definitely not a regex expert.

[anton-seaice]

Thats pretty odd, I guess you could test it by changing the order of patterns in AccessOm3Builder.PATTERNS[0] ?

[marc-white]

Thats pretty odd, I guess you could test it by changing the order of patterns in AccessOm3Builder.PATTERNS[0] ?

Yep, tried that. It always seemed to lock on to the "Y" pattern, regardless of where I put it.