Skip to content

Conversation

@peterdudfield
Copy link
Contributor

@peterdudfield peterdudfield commented Sep 2, 2025

Pull Request

Description

  • change Metoffice Global to have run times of 0 and 12
  • Set delay_minutes for Metoffice in ModelMetadata, different for UKV and global

#288

How Has This Been Tested?

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@peterdudfield peterdudfield requested a review from devsjc September 2, 2025 16:36
@peterdudfield peterdudfield changed the title No results Metoffice global options, and dont fail if not results Sep 3, 2025
@devsjc
Copy link
Collaborator

devsjc commented Sep 4, 2025

Can I just check the motivation for this?

  • Do we not want to fail if nothing is processed?
  • Are we pulling two different models from the MetOffice Datahub repository with different running hours for the same region?

I'm just wondering why it isn't a straight update to the india model implementation in the mo_datahub repository to say it's only running at 0 and 12. Having both implementations of the model is only necessary if we're pulling both from the same repository, which I don't think we are? But maybe my knowledge is behind the times...

@peterdudfield
Copy link
Contributor Author

Can I just check the motivation for this?

  • Do we not want to fail if nothing is processed?
  • Are we pulling two different models from the MetOffice Datahub repository with different running hours for the same region?

I'm just wondering why it isn't a straight update to the india model implementation in the mo_datahub repository to say it's only running at 0 and 12. Having both implementations of the model is only necessary if we're pulling both from the same repository, which I don't think we are? But maybe my knowledge is behind the times...

  • I think I can remove the change if nothing is processed.

  • Yea, we seem to have Metoffice global and UKV. They have different run times and delays.

@devsjc
Copy link
Collaborator

devsjc commented Sep 4, 2025

So this line covers UKV, and these two lines cover the global india and uk crops. If its a matter of the global india runtime being different, shouldn't we just add the with_running_hours to the existing india global model implementation?

So we'd end up with:

available_models={
    "default": entities.Models.MO_UM_GLOBAL_10KM.with_region("india"),
    "um-global-10km-india": entities.Models.MO_UM_GLOBAL_10KM.with_region("india").with_running_hours([0, 12]),
    "um-global-10km-uk": entities.Models.MO_UM_GLOBAL_10KM.with_region("uk"),
    "um-ukv-2km": entities.Models.MO_UM_UKV_2KM_LAEA,
},

or, even better, if both the uk and india global model implementations are actually run on this lower cadence, just update it in the model medatada directly?

@peterdudfield
Copy link
Contributor Author

So this line covers UKV, and these two lines cover the global india and uk crops. If its a matter of the global india runtime being different, shouldn't we just add the with_running_hours to the existing india global model implementation?

So we'd end up with:

available_models={
    "default": entities.Models.MO_UM_GLOBAL_10KM.with_region("india"),
    "um-global-10km-india": entities.Models.MO_UM_GLOBAL_10KM.with_region("india").with_running_hours([0, 12]),
    "um-global-10km-uk": entities.Models.MO_UM_GLOBAL_10KM.with_region("uk"),
    "um-ukv-2km": entities.Models.MO_UM_UKV_2KM_LAEA,
},

or, even better, if both the uk and india global model implementations are actually run on this lower cadence, just update it in the model medatada directly?

how do we cover different delay hours?

@devsjc
Copy link
Collaborator

devsjc commented Sep 4, 2025

Apologies for potentially being slow on this - different delay hours between what and what exactly?

If you're meaning different delay hours between the global and the ukv models, then we cover that as in the second example above. If you mean different delay hours between the india global and uk global specifically, then we cover that as in the first example above.

If however, you mean that we are pulling the same crop of a model, from the same repository, with different delays, then we'll have to implement something like what you've written in the PR. I just wanted to confirm that this was actually what was happening, because as far as I'm aware, we don't do this - unless something has changed recently.

In the issue description, you've just put

It looks like the metoffice global data delay is 5 hours, compared to 2 hours for the metoffice UKV

which sounds to me like there is only a difference between the global and the UKV models. So we'd do the second example above. But perhaps I'm misunderstanding!

@peterdudfield
Copy link
Contributor Author

Apologies for potentially being slow on this - different delay hours between what and what exactly?

If you're meaning different delay hours between the global and the ukv models, then we cover that as in the second example above. If you mean different delay hours between the india global and uk global specifically, then we cover that as in the first example above.

If however, you mean that we are pulling the same crop of a model, from the same repository, with different delays, then we'll have to implement something like what you've written in the PR. I just wanted to confirm that this was actually what was happening, because as far as I'm aware, we don't do this - unless something has changed recently.

In the issue description, you've just put

It looks like the metoffice global data delay is 5 hours, compared to 2 hours for the metoffice UKV

which sounds to me like there is only a difference between the global and the UKV models. So we'd do the second example above. But perhaps I'm misunderstanding!

Different delay hours between the global and the ukv models, sorry i missed how this was solved

@devsjc
Copy link
Collaborator

devsjc commented Sep 4, 2025

In that case, we just need to update this line:

to the new running hours.

@peterdudfield
Copy link
Contributor Author

In that case, we just need to update this line:

to the new running hours.

but the UKV nwp consumer, does need running hours of [0,6,12,18]

nwp-consumer/src/nwp_consumer/internal/entities/modelmetadata.py

ah nice. Ok ill update it then. What about the difference in delays thats currently here

@devsjc
Copy link
Collaborator

devsjc commented Sep 4, 2025

but the UKV nwp consumer, does need running hours of [0,6,12,18]

The UKV model metadata has seperate defined running hours to the global one:

running_hours=list(range(0, 24, 6)),

Ah I see. That's the delay in the dissemination, i.e. how long it takes after the init time of the model until it is available on datahub. Is this different for the two models?

@peterdudfield
Copy link
Contributor Author

but the UKV nwp consumer, does need running hours of [0,6,12,18]

The UKV model metadata has seperate defined running hours to the global one:

running_hours=list(range(0, 24, 6)),

Ah I see. That's the delay in the dissemination, i.e. how long it takes after the init time of the model until it is available on datahub. Is this different for the two models?

yea, metoffice UKV I think is 2 hours, where as global is more like 5 hours.

@devsjc
Copy link
Collaborator

devsjc commented Sep 4, 2025

Ah okay, so we should probably move that to be a property on the model metadata instead of the repository metadata then! Then it can be varied per model

@peterdudfield
Copy link
Contributor Author

Ah okay, so we should probably move that to be a property on the model metadata instead of the repository metadata then! Then it can be varied per model

shall i have a go at moving it over?

@peterdudfield
Copy link
Contributor Author

Although its perhaps not as simple as I thought, as ECMWF_realtime, and ECMWF_mars use the same entitles.Models, but have different delay_minutes

@peterdudfield peterdudfield marked this pull request as draft September 4, 2025 17:47
@peterdudfield peterdudfield marked this pull request as ready for review September 5, 2025 13:54
@devsjc
Copy link
Collaborator

devsjc commented Sep 15, 2025

I understand now that the models themselves have to have the ability to have different delays. As such I think the best course of action is to:

  • Move the delay_mins parameter from RawRepositoryMetadata to ModelMetadata
  • Add a with_delay_mins function to the ModelMetadata class akin to the other with_* builder functions
  • Update determine_latest_it_from in consumer_service.py to take an optional delay_mins (default to 0) that is passed through from the model metadata at the calling of the function

This will also require updating the tests!

@peterdudfield
Copy link
Contributor Author

Done in #293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants