MetOffice / CSET

Toolkit for evaluation and investigation of numerical models for weather and climate applications.
https://metoffice.github.io/CSET/
Apache License 2.0
10 stars 4 forks source link

Generate Histograms for 2D field #594

Closed Sylviabohnenstengel closed 2 months ago

Sylviabohnenstengel commented 4 months ago

Description

generating a PDF for a 2D field looping over vertical levels

Fixes #453

Contribution checklist

Aim to have all relevant checks ticked off before merging. See the developer's guide for more detail.

Sylviabohnenstengel commented 4 months ago

adding ensemble capability

jfrost-mo commented 3 months ago

Old branch head: 2d9ead7edb58ee1338ec2ff506ba324688014746

github-actions[bot] commented 3 months ago

Coverage

jfrost-mo commented 3 months ago

Note to self: There are a bunch of conflict markers in this branch's code that need cleaning up.

Sylviabohnenstengel commented 3 months ago

rebased the branch

Sylviabohnenstengel commented 3 months ago

code working and now need to add unit test. image

Sylviabohnenstengel commented 3 months ago

GitHub Copilot used to debugfilename in assert Path("test_473718.0.png").is_file()

Sylviabohnenstengel commented 3 months ago

@daflack @jfrost-mo I suggest that James does the technical review first and once that has passed David can do the science review.

Sylviabohnenstengel commented 2 months ago

@jfrost-mo @daflack Many thanks for your reviews. I have addressed them all. Examples of line style plots are above in the respective comments. I added a trigger field in rose-meta to allow the user to configure the histogram type. Selection of postage stamps or all members in a single plot is not configurable in rose meta, but instead fixed in the yaml recipe file.

@jfrost-mo it would be good if you had another look at the test_plot.py @daflack I leave the bin width to the hist function at the moment. testing over the summer will show if we want to constrain that further to have a smoother pdf curve instead.

Sylviabohnenstengel commented 2 months ago

Happy with changes from science perspective and agree with where you have left things to then potentially be implemented with a change later.

My one thought, and happy if you do not implement it at this stage (which is why I have made the comment here rather than in the code), is whether there should be clearer indication of the different options available for $HISTTYPE. Either by linking to plt.hist in the documentation for histtype or by listing the different options. At the moment it is a little unclear on what str options can be put in.

Good point @daflack I have added more help text to rose-meta, plot.py and the recipe file. This is the change: 3466745..f092b07

Sylviabohnenstengel commented 2 months ago

@jfrost-mo over to you to take a final look. Thanks both @daflack and @jfrost-mo for the review :-)

Sylviabohnenstengel commented 2 months ago

Plotting code changes look good. One of the include files has an unnecessary loop, and more tests need writing for the new plotting functionality.

It has the wrong statement in the loop. IT should read: {% for model_field in PRESSURE_LEVEL_MODEL_FIELDS %}

Corrected now in 11fe6d7..8344e50

Sylviabohnenstengel commented 2 months ago

Plotting code changes look good. One of the include files has an unnecessary loop, and more tests need writing for the new plotting functionality.

This has been changed in the code and this version is outdated.

jfrost-mo commented 2 months ago

Test coverage look reasonable. There are still a few lines added that are uncovered, but it'll do for now.