Add API to support wind grid display. #13
Conversation
| hgt_3d = np.array([g.values for g in hgt_grbs]) | ||
| u_3d = np.array([g.values for g in u_grbs]) | ||
| v_3d = np.array([g.values for g in v_grbs]) | ||
| t_3d = np.array([g.values for g in t_grbs]) - 273.15 |
There was a problem hiding this comment.
@cameronnegrete if you could add context as to this and all the other constants used throughout the file that would be awesome.
There was a problem hiding this comment.
I agree, what is this and the other magic numbers?
There was a problem hiding this comment.
GRIB files present the temps in KELVIN because Kelvin is the thermodynically correct unit to use where absolute 0 is where the atoms have no more energy. Temp-273.15 converts from kelvin to degree C.
CrazyKidJack
left a comment
CrazyKidJack
left a comment
There was a problem hiding this comment.
Looks pretty good, though a few questions.
Main question is: why is this in the API? We should strive to keep as much of the code in the frontend as possible. Is it here just to reduce the number of API calls to the wind data provider? If so, do we have a precedent for external API calls for other types of data being centralized here?
| } | ||
| with data_lock: | ||
| weather_data['wx_state'] = new_state | ||
| weather_data['last_updated_utc'] = processed_at |
There was a problem hiding this comment.
Do we need to store processed_at twice?
| top_lon = float(request.args.get("toplong")) | ||
| bottom_lat = float(request.args.get("bottomlat")) | ||
| bottom_lon = float(request.args.get("bottomlong")) | ||
| fl = int(request.args.get("fl", 300)) # default FL300 |
There was a problem hiding this comment.
Does it actually make sense to provide a default here? Or should we require that consumers specify what flight level they care about?
| bottom_lon = float(request.args.get("bottomlong")) | ||
| fl = int(request.args.get("fl", 300)) # default FL300 | ||
|
|
||
| with data_lock: |
There was a problem hiding this comment.
Why do we need to hold this lock the whole time?
As far as I can tell, we should only need to hold it for lines 51-53 right?
| "temperature_c": int(levels[fl]["temp"][i, j]), | ||
| }) | ||
| with data_lock: | ||
| metadata = weather_data.get("wx_state") |
There was a problem hiding this comment.
why not retrieve this at the same time as the lats, lons, and levels so as to prevent needing the lock multiple times?
| if response.status_code == 200: | ||
| with open(save_path, 'wb') as f: | ||
| for chunk in response.iter_content(chunk_size=8192): | ||
| f.write(chunk) |
There was a problem hiding this comment.
is there a reason to actually save the file to disk instead of just holding the data in memory?
| download_rap_grib2(date_str, cycle_hour, forecast_hour, save_path) | ||
| interpolate_uv_temp_at_flight_levels(save_path) | ||
| processed_at = datetime.now(timezone.utc).isoformat() | ||
| new_state = { |
There was a problem hiding this comment.
This dictionary / structure should be a class so that its members could be accessed via attributes instead of via string keys
|
|
||
| # thread-safe global storage | ||
| data_lock = threading.Lock() | ||
| weather_data = { |
There was a problem hiding this comment.
This dictionary should be a class so that its members can be accessed via attributes instead of string keys
| return date_str, cycle_hour, forecast_hour, save_path | ||
|
|
||
| # Function to check if new forecast hour differs from previously stored state | ||
| def check_state(new_forecast): |
There was a problem hiding this comment.
Please change the name of this function and parameter to something more descriptive
| def check_state(new_forecast): | |
| def different_hour(new_hour): |
| # Wrapper function to execute the full workflow conditionally based on forecast state | ||
| def run_grid(): | ||
| date_str, cycle_hour, forecast_hour, save_path = get_date() | ||
| state_check = check_state(forecast_hour) |
There was a problem hiding this comment.
As with the other comment about the function name...
please change the name of this variable to something more descriptive
| state_check = check_state(forecast_hour) | |
| need_new_file = diff_hour(forecast_hour) |
| interpolate_uv_temp_at_flight_levels(save_path) | ||
| # persist new state in memory (add processed timestamp) | ||
| processed_at = datetime.now(timezone.utc).isoformat() | ||
| new_state = { |
There was a problem hiding this comment.
This dictionary should be a class so that its members can be accessed via attributes instead of string keys
CrazyKidJack
left a comment
CrazyKidJack
left a comment
There was a problem hiding this comment.
In addition to the previous comments,
please review whether anonymous dictionary types should be converted to proper named types
The download and processing of the RAP GRIB file is memory intensive enough that it should not be done in the client, hence the API here. Because we store the processed wind-data in memory the load on the API is low once processing is complete. |
|
So the commented out stuff for writing to a CSV was with the initial thought that IRL, the RAP wind and temp data is actually also used by TFMS to calculate trajectories for TBFM in addition to EDST trajectory modeling. I wrote it in there just incase Kyle over at simtraffic wanted to grab the data for his project use. It can easily be cut if it wont be used, hence the commenting out. |
|
|
||
| lats, lons = hgt_grbs[0].latlons() | ||
|
|
||
| hgt_ft = hgt_3d * 3.281 |
| lats, lons = hgt_grbs[0].latlons() | ||
|
|
||
| hgt_ft = hgt_3d * 3.281 | ||
| fl_3d = hgt_ft / 100.0 |
This adds an API endpoint that returns a down-selected dataset of current upper wind conditions (speed, direction, temp) within the queried flight level and bounds. This uses GRIB data taken from NOAA's RAP which is what the real EDST uses. This data is automatically updated at 5 minutes past the top of the hour, and held in RAM for rapid recall by the API.