Skip to content

Output grib files - #149

Merged
havardhhaugen merged 16 commits into
metno:mainfrom
fmidev:upstream
Aug 21, 2025
Merged

Output grib files#149
havardhhaugen merged 16 commits into
metno:mainfrom
fmidev:upstream

Conversation

@osmundi

@osmundi osmundi commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

Possibility to write grib files from predictions.

routing:
    ...
    outputs:
      - netcdf:
          ...
      - grib:
          filename_pattern: ${output}/pred_%Y%m%dT%HZ.grib

NOTE: The lat/lon of first grid point are hard coded to MEPS domain currently

@ways

ways commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

Thank you, @osmundi

prospector has some complaints, like:

bris/outputs/grib.py
  Line: 1
    pylint: unused-import / Unused import sys
  Line: 2
    pylint: unused-import / Unused import os
  Line: 3
    pylint: unused-import / Unused timedelta imported from datetime
  Line: 6
    pylint: unused-import / Unused import gridpp
  Line: 8
    pylint: unused-import / Unused xarray imported as xr
  Line: 56
    pylint: no-else-return / Unnecessary "else" after "return", remove the "else" and de-indent the code inside it (col 8)
...

To run prospector, use tox -e prospector.

Can you also supply a test for outputting to grib in the form of a new tox test, please?

@ways ways self-assigned this Jun 5, 2025
@ways

ways commented Jul 2, 2025

Copy link
Copy Markdown
Contributor

@osmundi Will you let me know when this is ready for review and merge?

@ways

ways commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Tried adding this to the tox test "inference_CI" like this:

config/tox_test_inference.yaml:

# Minimal config for running inference on CPU. See https://github.com/metno/bris-inference/wiki/Tests

start_date: 2022-01-01T00:00:00
end_date: 2022-01-02T00:00:00

checkpoints:
  forecaster:
    checkpoint_path: ./inference-last.ckpt
    leadtimes: 2

dataset: ./bris_random_data.zarr
frequency: 6h
workdir: /tmp/

dataloader:
  datamodule:
    _target_: bris.data.dataset.NativeGridDataset
    _convert_: all

hardware:
  num_gpus_per_node: 1
  num_gpus_per_model: 1
  num_nodes: 1

release_cache: False

model:
  _target_: bris.model.brispredictor.BrisPredictor
  _convert_: all

routing:
  - decoder_index: 0
    domain_index: 0
    domain: 0
    outputs:
      - netcdf:
          filename_pattern: ./tox_test_inference.nc
          variables: [2t, 2d]
      - grib:
          filename_pattern: ./tox_test_inference.grib

Unfortunately that fails with:

[rank0]:   File "/home/larsfp/src/bris-inference/.tox/inference_CI/lib/python3.12/site-packages/bris/outputs/grib.py", line 69, in _add_forecast
[rank0]:     self.write(filename, times, pred[..., None])
[rank0]:   File "/home/larsfp/src/bris-inference/.tox/inference_CI/lib/python3.12/site-packages/bris/outputs/grib.py", line 104, in write
[rank0]:     self.pm.field_shape[1],
[rank0]:     ~~~~~~~~~~~~~~~~~~~^^^
[rank0]: IndexError: tuple index out of range

Can you check what's required to get it working? Perhaps a new separate tox test is required.

@osmundi

osmundi commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

Now the full test suite should be working.

@ways

ways commented Aug 18, 2025

Copy link
Copy Markdown
Contributor

I have little knowledge of grib, but trying to check if the file is valid I am able to list, but not "get data":

> grib_ls  ./.tox/inference_CI/tmp/tox_test_inference.grib 
./.tox/inference_CI/tmp/tox_test_inference.grib
edition      centre       date         dataType     gridType     stepRange    typeOfLevel  level        shortName    packingType  
2            ecmf         20220102     af           lambert      0            heightAboveGround  2            2d           grid_ccsds  
2            ecmf         20220102     af           lambert      0            heightAboveGround  2            2t           grid_ccsds  
2            ecmf         20220102     af           lambert      6            heightAboveGround  2            2d           grid_ccsds  
2            ecmf         20220102     af           lambert      6            heightAboveGround  2            2t           grid_ccsds  
4 of 4 messages in ./.tox/inference_CI/tmp/tox_test_inference.grib
> grib_get_data ./.tox/inference_CI/tmp/tox_test_inference.grib
ECCODES ERROR   :  Lambert conformal Geoiterator: Wrong number of points (162!=0x162)
ECCODES ERROR   :  Geoiterator factory: Error instantiating iterator lambert_conformal (Grid description is wrong or inconsistent)
ECCODES ERROR   :  Grid description is wrong or inconsistent

Is this expected with the test data?

@osmundi

osmundi commented Aug 18, 2025

Copy link
Copy Markdown
Contributor Author

I'll check it out.

@ways

ways commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

I'll check it out.

Seems OK now. Shall I merge or do you have more work to do?

@osmundi

osmundi commented Aug 20, 2025

Copy link
Copy Markdown
Contributor Author

Sure, I dont have anything else for now. I'll make new PR if we are going to add some new functionality to it.

@ways
ways requested a review from havardhhaugen August 20, 2025 07:40
@havardhhaugen

Copy link
Copy Markdown
Contributor

@osmundi Can you merge / rebase main into this branch so it's up to date (with metno:main), and then check that everything works on your end? I hope this won't be too painful.

We have merged functionality to run ensemble models now, and as far as I can tell that is not supported in the grib output at the moment. Up to you if we fix that now or in a later PR, but we should probably throw an error if someone tries to run an ensemble model with grib output if it doesn't work.

@osmundi

osmundi commented Aug 20, 2025

Copy link
Copy Markdown
Contributor Author

Rebasing done and the full test suite is at least ok with tox.

@havardhhaugen havardhhaugen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good now, thanks for the contribution! Will set up a separate issue / PR to fix the issue with shared variables and ensemble members.

@havardhhaugen
havardhhaugen merged commit a3dc099 into metno:main Aug 21, 2025
1 check passed
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.

4 participants