Skip to content

feat(input): add FDB input class #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 30, 2025
Merged

Conversation

frazane
Copy link
Contributor

@frazane frazane commented Mar 24, 2025

Description

This PR introduces a new class to get input fields from FDB. For using it, one must specify some keys of the request as kwargs in the configuration, which are then completed/overridden by the keys (date, time, param, etc.) of the mars request for the data as specified in the checkpoint (just like in MarsInput). The FDB configuration can either be provided as additional key-value pairs, or as environment variables.

Example config

input:
    fdb:
      expver: 0001
      number: 0
      step: 0
      class: od
      levtype: sfc
      model: ICON-CH1-EPS
      stream: enfo
      type: ememb

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Issue Number

Code Compatibility

  • I have performed a self-review of my code

Code Performance and Testing

  • I have added tests that prove my fix is effective or that my feature works
  • I ran the complete Pytest test suite locally, and they pass

Dependencies

  • I have ensured that the code is still pip-installable after the changes and runs
  • I have tested that new dependencies themselves are pip-installable.

Documentation

  • My code follows the style guidelines of this project
  • I have updated the documentation and docstrings to reflect the changes
  • I have added comments to my code, particularly in hard-to-understand areas

Additional Notes

Copy link
Collaborator

@b8raoult b8raoult left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not need the "request_template" part. This will be more inline with the other inputs and datasets sources

@frazane
Copy link
Contributor Author

frazane commented Mar 25, 2025

You do not need the "request_template" part. This will be more inline with the other inputs and datasets sources

Hi @b8raoult, my understanding is that fdb requests must be fully specified (is that correct? this seems to be the case at least with our fdb setup). However, we do not get a fully specified request from the checkpoint alone, and some missing keys need to be completed somehow. A request template seemed like the most straightforward approach but if there is a better way we should discuss it.

@frazane
Copy link
Contributor Author

frazane commented Apr 3, 2025

Removed the request_template - now additional keys are passed as kwargs.

@frazane frazane marked this pull request as ready for review April 3, 2025 16:34
@frazane frazane requested a review from b8raoult April 7, 2025 16:23
@frazane frazane force-pushed the feature/fdb-input branch from e362413 to 4a41ae4 Compare April 29, 2025 08:04
@frazane frazane requested a review from HCookie April 29, 2025 08:04
Copy link
Member

@HCookie HCookie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good.
@frazane Happy for this to be merged?

@frazane
Copy link
Contributor Author

frazane commented Apr 30, 2025

Yes. Thanks for the support!

@HCookie HCookie merged commit ca6d37f into ecmwf:main Apr 30, 2025
17 checks passed
@github-project-automation github-project-automation bot moved this from Now In Progress to Done in Anemoi-dev Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants