Skip to content
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

Oceans674 #689

Merged
merged 13 commits into from
Jul 6, 2022
Merged

Oceans674 #689

merged 13 commits into from
Jul 6, 2022

Conversation

lewisblake
Copy link
Member

@lewisblake lewisblake commented Jun 28, 2022

You've heard of Ocean's 11, Ocean's 12, and Ocean's 13. Well get ready for....

Addressing issues described in #674 and this comment: #582 (comment)

We know that almost all regions are contained in "Oceans". This is because the the HTAP definition in regions_def.py. A first observation is that on the website, when you select "North America", "Pacific, Australia, and New Zealand", or "Russia, Ukraine, Belarus", stations on the globe do not "light up" as they do for the rest of the region (e.g., Europe, Eastern Europe, etc.). I suspected this might have to do with the face that each of these regions have longitude boundaries in which the easterly-most coordinate is "greater than" the westerly-most coordinate (i.e., the bounding box goes through the antipodes of the prime meridian), Sure enough, the function contains_coordinate was not set up for this case. So I changed it, and added some tests.

This needs testing on a run, which I will try tomorrow. Not sure if it will correct everything in the issue, but this is a start.

@codecov
Copy link

codecov bot commented Jun 28, 2022

Codecov Report

Merging #689 (b4e6972) into main-dev (5ce043c) will increase coverage by 0.05%.
The diff coverage is 94.11%.

@@             Coverage Diff              @@
##           main-dev     #689      +/-   ##
============================================
+ Coverage     76.81%   76.86%   +0.05%     
============================================
  Files            97       97              
  Lines         17536    17548      +12     
============================================
+ Hits          13471    13489      +18     
+ Misses         4065     4059       -6     
Flag Coverage Δ
unittests 76.86% <94.11%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pyaerocom/region.py 70.28% <94.11%> (+7.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ce043c...b4e6972. Read the comment docs.

@lewisblake
Copy link
Member Author

lewisblake commented Jun 29, 2022

Q: Also, why is there no "W Europe": https://aeroval.met.no/overall.php?project=cams2-82&exp_name=IFS&station=WORLD&tab=timeseries#

A: Not in _HTAP_NAMES

@lewisblake
Copy link
Member Author

@AugustinMortier Are the regions on the website defined from regions.json?

@lewisblake
Copy link
Member Author

Here's how I see the situation.

The problem is that the "Ocean" region is applied to many of the stations. However, the statistics (e.g., time series) for each region appear to be computed correctly. Indeed, they are. This is because the filtering for a colocated data object takes place in colocateddata.py::apply_region_mask(). This function accepts as an argument region_id (str) and calls a function load_region_mask_xr, which opens the HTAP NetCDF filters (e.g., EAShtap.0.1x0.1deg.nc located at https://pyaerocom.met.no/pyaerocom-suppl/htap_masks/), and returns them as xarray objects. The region_id corresponds to keys in _HTAP_NAMES provided in region_defs.py. Basically the same key is used to implement the NetCDF filtering as is used to get the region definition in _HTAP_DEFS, which end up in regions.json and then on the web interface. It is also important to note that regions.json has essentially a dictionary, with each region defined in terms of rectangular bounds:

"N America": {
        "minLat": 18.95,
        "maxLat": 71.15,
        "minLon": 172,
        "maxLon": -52
    },

If the regions are are stored as an array (#580), then coldatatojson_helpers.py::_get_stat_regions() and coldatatojson_helpers.py::_process_sites() will get multiple matches against the regions definitions in _HTAP_DEFS.

Therefore, we either need to completely re-overhaul how regions are defined on the web interface, or on the web, we only need to plot points for which "Oceans" are their only region. However, having the regions defined in terms of the NetCDF filters and also be flexible enough to be an array is fundamentally incompatible.

This was linked to issues Jun 30, 2022
@lewisblake lewisblake requested a review from dulte June 30, 2022 11:40
@lewisblake
Copy link
Member Author

Q: Also, why is there no "W Europe": https://aeroval.met.no/overall.php?project=cams2-82&exp_name=IFS&station=WORLD&tab=timeseries#

A: Not in _HTAP_NAMES

@AugustinMortier do you want W Europe on the web as well?

@lewisblake lewisblake requested review from avaldebe and removed request for dulte July 5, 2022 14:13
@lewisblake lewisblake marked this pull request as ready for review July 5, 2022 14:13
Copy link
Collaborator

@avaldebe avaldebe left a comment

Choose a reason for hiding this comment

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

🏁 🏇🏼
maybe test_get_regions_coord_with_supplied_regions_dict could do with some clean up, but it is a very minor nitpick.
Please, merge when you see fit.

@lewisblake lewisblake merged commit 367d5cb into main-dev Jul 6, 2022
@lewisblake lewisblake deleted the oceans674 branch July 6, 2022 06:58
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.

Oceans region added to too many stations Missing regions
2 participants