NOAA-CSL / MELODIES-MONET

MELODIES MONET - diagnostic tool for evaluating models against a variety of observations including surface, aircraft, and satellite data all within a common framework
https://melodies-monet.readthedocs.io
Apache License 2.0
19 stars 21 forks source link

Add RAQMS reader to MELODIES MONET #107

Closed rschwant closed 1 year ago

rschwant commented 1 year ago

Add RAQMS reader to MONETIO. Update driver and ReadTheDocs in MELODIES MONET to use the new RAQMS reader.

zmoon commented 1 year ago

xref: https://github.com/NOAA-CSL/MELODIES-MONET/pull/99 https://github.com/noaa-oar-arl/monetio/pull/85

mebruckner commented 1 year ago

Am working to finish checking changes as well as update the documentation. I ran into a potential issue regarding plots. Within surfplots for the spatial plots (make_spatial_bias and make_spatial_overlay), the default lat/lon minimum and maximum values for domain_type == all is a CONUS bounding. @zmoon how should I proceed? Should this be changed, or for any model is a domain_type of all just Conus and something else be used to push to the else statement in the map generation?

zmoon commented 1 year ago

@mebruckner that is a good point, probably this should be updated so that "all" is actually all[^a] and we can have a separate "CONUS" option. We can open a separate issue for this.

I think currently if you just don't set domain_type (just delete the line in your config), you should get the behavior I think you are looking for (the else branch).

Edit: actually looks like it has to be set currently or the driver will fail, try just setting it to something random, like domain_type: ["asdf"]. https://github.com/NOAA-CSL/MELODIES-MONET/blob/abb0354116ef0f40119815b983c1bd7edc2c5672/melodies_monet/driver.py#L656

Edit2: actually that won't work either due to https://github.com/NOAA-CSL/MELODIES-MONET/blob/abb0354116ef0f40119815b983c1bd7edc2c5672/melodies_monet/driver.py#L738-L739 so I guess you'll just have to temporarily hack the "all" block in the plotting function if you want to see all.

[^a]: Though some of the obs datasets we use are essentially limited to CONUS.

mebruckner commented 1 year ago

Thanks- I'd already worked around it in my personal scripts and codebase. It doesn't actually cause any issues in generating plots, I just came across it again while ensuring RAQMS is included in the plot setups. There just might be some user confusion if the obs and model sets include points outside the spatial plotting domain which are then included in the other plots and statistics (which does happen with the current all behavior).

Documentation for the RAQMS addition will be complete before the weekend, and I will push the new commits for approval.

zmoon commented 1 year ago

It doesn't actually cause any issues in generating plots,

Except that if there are points outside the CONUS domain you wouldn't see them, yes?

But your other point about the averages and stats seems the more important, I will open issue.

mebruckner commented 1 year ago

Yes, any points outside the CONUS domain just wouldn't be visible in the spatial plots when the user gives domain_type == all under the current construction due to the choice of minimum and maximum lat/lons for the plot extent.

mebruckner commented 1 year ago

In running my test notebook I ran into an issue with the aeronet pairing that arises from the aod field in RAQMS not having a z-dimension. I had bypassed this issue by adding a try/execpt that would bypass the z issue for 2d model fields. This could be a special case sort of situation, so I don't know the to handle it.

zmoon commented 1 year ago

hmm, we could add a surface-only option to the reader like the WRF-Chem one has?

zmoon commented 1 year ago

@mebruckner I added a surf_only option in a branch here if you want to try it https://github.com/zmoon/monetio/tree/raqms with something like

model:
  RAQMS:
    mod_type: raqms
    mod_kwargs:
      surf_only: true

That should give every data var a singleton z dimension that can be selected.

mebruckner commented 1 year ago

Zach, thanks for adding this option. It does work.

rschwant commented 1 year ago

This is now in the develop branch of MELODIES MONET and complete: https://github.com/NOAA-CSL/MELODIES-MONET/pull/147. I'm closing this issue.