-
Notifications
You must be signed in to change notification settings - Fork 95
tutorial: replace deprecated DataPoint API #7044
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
base: master
Are you sure you want to change the base?
Conversation
| RESOLUTION = 0.2 | ||
| # The area to generate forecasts for (lng1, lat1, lng2, lat2) | ||
| DOMAIN = -12,48,5,61 # Do not change! | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I'm not convinced -
d109fc3 to
f684f1d
Compare
| # TODO - work out why this isn't working | ||
| scale = get_scale(domain, width) | ||
| offset = get_offset(domain, scale) | ||
| scale = (1672.2334443981335, 3344.466888796267) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliver-sanders - These numbers seem to work (roughly) might I poke you to have a look at why the functions do or don't?
If you run the workflow in debug mode you get piuctures
Compare my images
Actually, only get_offset isn't giving a sensible answer.
NVM - I'll keep poking - I think that this is a problem with the projection.
| [y[1] for y in z_coords], | ||
| color='red') | ||
| plt.savefig('wind.png') | ||
| plt.savefig(f'{os.environ["CYLC_TASK_LOG_DIR"]}/wind.png') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're only saving this for people to look at, so it ought to go here?
| return req.json()['SiteRep']['DV']['Location'] | ||
|
|
||
|
|
||
| def get_archived_data(site_id, time): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see any evidence of actual archived data - Perhaps we should save some at some point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are using archive data as part of the Rose tutorial. It gets bundled with the app for use in the test config.
I think this test data will need to be re-generated from the new data source (check if this is true) otherwise the Rose app might fail due to a resolution mismatch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wierdly no, -we're using archived output of this task for the forecast task. This looks like legacy. Still need a rose PR to follow up this though. PR at metomi/rose#2959
I think we should have a follow up issue to check and fix.
cylc/flow/etc/tutorial/cylc-forecasting-workflow/bin/get-observations
Outdated
Show resolved
Hide resolved
|
@wxtim, FYI, this will need to go into a bugfix release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR is on the wrong branch.
Some small comments so far:
cylc/flow/etc/tutorial/cylc-forecasting-workflow/bin/get-observations
Outdated
Show resolved
Hide resolved
| # A template Map file | ||
| MAP_FILE = "${CYLC_TASK_LOG_ROOT}-map.html" | ||
| # Create the html map file in the task's log directory. | ||
| MAP_FILE = "${CYLC_TASK_LOG_ROOT}-map.html" | ||
| # The path to the template file used to generate the html map. | ||
| MAP_TEMPLATE = "$CYLC_WORKFLOW_RUN_DIR/lib/template/map.html" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't need to specify these here before.
Might be easier to stick with using the work or share directory as appropriate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because I thought that it was quite nice to view the intermediate stages from Cylc Review. Happy to remove if you think I should though.,
cylc/flow/etc/tutorial/cylc-forecasting-workflow/lib/python/util.py
Outdated
Show resolved
Hide resolved
|
It looks like we have multiple options for reading HDF5 files. h5py, is one, but Pandas and xarray are apparently alternatives. Pandas might be an appealing option as widely used and is already an optional dependency. Can we have a quick review of the options to work out which is the most lightweight, easiest to support, least likely to cause problems, etc. |
I think that they may all be the same: The documentation certainly suggests that pandas.read_hdf uses pytables, whose docs suggest that uses h5py. Looking at the source of pandas shows h5py is an optional depency. I think it's going to be h5py whatever we choose. |
|
The Note: It uses ~25% CPU (but negligible RAM) on my box. |
| 'NNW': '157.5' | ||
| } | ||
|
|
||
| class Meteorology: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably a bit bonkers. I won't mind (very much) if you tell me to take it out again.
Remove unwated ref to datapoint API key remove all references to datapoint api keys Remove api keys from cylc get-resources Update conda-environment.yml Co-authored-by: Oliver Sanders <[email protected]>
Add a meteorological tweak remove unwanted env vars
| dim_x, dim_y, resolution, resolution, | ||
| spline_level) | ||
|
|
||
| domain = util.parse_domain(os.environ['DOMAIN']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems odd to run this every single loop.



Closes #6053
Datapoint is to be switched off from the first of December.
The data is available from the Met Office via the Amazon Sustainability Data Initiative. Amazon link.
Annoyingly the metadata there seems quite limited, so I applied to a bunch of contacts in Obs R&D who insisted that the data has the following properties:
Which raises 2 problems:
Rather than re-jigging the mathematics I've made something vaguely plausible by fiddling with domain values. Hopefully this creates a product good enough for the training purpose for which it is built.
Finally, I have fixed a bug I introduced when I added the SYNOP collecting routine - these wind observations are in meteorological convention (wind is blowing from), but we need where the wind is going to, so all wind directions were 180° off!
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).?.?.xbranch.