ncsco / pinemap-dss-help

Issue tracker for PINEMAP DSS
0 stars 0 forks source link

Clicking a point with no data should return an error #9

Closed daviswx closed 7 years ago

daviswx commented 9 years ago

Currently, clicking a point over the ocean returns an error, but there are other land-based locations (like over Canada and Mexico, and some ragged edges along coastlines) that also don't have data. In some cases, clicking a point in these locations can return some wacky results, like a temperature of -18457.9F.

We need a bit of separate logic to check whether a clicked point has data, and if not, return an error instead of displaying these zero or non-zero extreme values.

daviswx commented 9 years ago

This is now fixed for the current tools on the development site. Clicking on Canada, Mexico, a ragged coastal area with no MACA data, or any other non-CONUS point should result in a "Data is unavailable for your selected location" message being displayed.

Implementing it wasn't quite as straightforward as I expected since the signs of missing data aren't always obvious. For instance, on the Extreme Min Temp Risk tool, many southerly locations (like in south Florida) have essentially 0 occurrences of temperatures < 32°F. So for this tool, instead of checking for zeroes, I'm checking for any -9999s or -19998s (-9999 × 2). If there are any of those, I consider that location to have an incomplete dataset, therefore I am returning the error.

Likewise for the seedling deployment tools, 0°F values are possible, so I'm checking for anything less than -100, which seems to take care of those -18457.9°F values (which, by the way, is -9999K converted to Fahrenheit).

And for the Summer Precip tool, since I'm converting from mm to inches, I'm checking for any values less than -393 inches (≈ -9999 mm), as well as any 0 values, since there shouldn't conceivably any locations with no summertime precipitation. If we expand the DSS to the moon, we'll have to re-think this logic. ;)

Anyway, that's implemented now so I'm closing this issue, but please re-open it if you notice any bugs with it.

hadinon commented 8 years ago

I am just now remembering to check in on this and found a few bugs...

1) When I click a location outside the US, it says, "Data unavailable" as expected. However, it changes from being highlighted in green to being highlighted in red on the Seedling Deployment tools. 2) Similarly, this occurs on the Extreme Min Temp tool except it lists the location then flashes to "Data unavailable" highlighted in green and finally, flashes the "Data unavailable" in red. Same for summer temp tool and summer precip tool. 3) This capability on the historical map and projected occurrence doesn't seem to work on the extreme min temp tool (e.g. says "0 days" when I clicked in Canada and Mexico) or the summer temp tool. Not an issue with summer precip tool.

Let me know if you are unable to replilcate

daviswx commented 8 years ago

Part (3) of this is now fixed. Apparently after I implemented these no-data errors previously, I went back and cleaned up my code a bit, which inadvertently broke them. Go figure!

Parts (1) and (2) are caused by a similar characteristic in the code:

So for cases (1) and (2), it found a valid location initially, like a territory in Mexico or one of the ragged coastal locations, but there was no MACA data for that point, so the location went from green to red.

I assume the best correction would be to check for location validity AND data availability before showing anything in the location legend. This would require some changes to the back-end code, and it also may create a slight lag between when the map is clicked and when the location shows up. However, I don't think it would be too bad, and it would definitely be smoother than having the legend go from green to red.

I'll start working on it and we can evaluate it from there.

hadinon commented 8 years ago

Thanks, Corey. Here are the items we discussed:

1) tweak error message for time series 2) create a consistent no data message 3) show marker over ocean 4) add loading icon for location check

daviswx commented 8 years ago

All four of these should now be fixed for the Summer Precipitation tool.

For (2) and (3), I removed the initial check of location validity, so there won't be a separate no-data message for clicking over the ocean vs. Canada vs. a ragged coastal location without a MACA point. This also means the marker will always appear.

For (1), I changed the time series error to reflect that no data is available. However, I'm wondering if this message is redundant since a similar message shows up in the location legend.

And for (4), I've added a loading icon to the location legend. I'm wondering what wording should accompany this. I have "Loading" now, but it's really not loading anything. It's just geocoding the clicked point, but this is probably TMI for the user. Any suggestions on better wording? I also considered something like "Calculating" or "Determining location", but I'm not happy with either of those.

I will also start working on implementing the new functionality for the other tools.

hadinon commented 8 years ago

Everything looks great!

I think the redundancy is good in this case just in case someone ONLY looks above the map or ONLY looks below the map. :)

As far as the wording for the loading icon, I don't have any heartburn over what you have there since it is technically "loading" the location details, right? Another text option would be, "Finding your location..." although that might be redundant, too. Thoughts?

hadinon commented 8 years ago

P.S. you could even do, "Loading your location"

daviswx commented 8 years ago

I like the wording "Finding your location..." so I went with that.

Also, this functionality is now working for the other climate risk tools. It's also mostly working for the seedling deployment tools, although I still need to fix a bug where clicking a no-data point will still display the temperature (as -18457.9) above the markers, and let you show the risk ranges for those points.

hadinon commented 8 years ago

Sounds good. Can you replicate this or is it just my silly computer?

image

daviswx commented 8 years ago

As we just discussed, this bug was tied to a zero value for a future time slice and 2 standard deviation below the mean. It should be fixed now, but more spot checks to confirm would be great.

daviswx commented 8 years ago

The seedling deployment tools have been updated so clicking a no-data point hides the isotherm layers and legend.

Also, I found another problem with the Extreme Min Temp tool that may cause "NaN days" to display for some sites, i.e., over the Florida keys. I'll work on it.

daviswx commented 8 years ago

I think all of the no-data errors are fixed and working now, so I am closing this issue. Feel free to re-open it if you find others.

This may also be a good area for our office's alpha testers to explore, particularly clicking around the edges of the MACA grid.

hadinon commented 8 years ago

The bug with the 0 value for future time slice and 2 std dev below the mean looks to be fixed. However, I just noticed this.... it appears that there is no MACA data at that point but there is a historical average value at that point (from the Idaho dataset). How do we want to handle that situation? Oddly enough, this appears to only occur with the extreme min temp tool image

On the summer temperature tool, this does not occur. However, there is data at those locations on the historical map and yet it says, "Data is unavailable" because there's no future data available at that point. Do you think this is confusing? I'm not sure the best way to handle this. Thoughts? image

Also, I tested out the seedling deployment tool in terms of the "no data" bug you fixed and the isotherms still don't go away when I clicked a valid location, hit "show 5 degree range", and then clicked over the ocean. image

Lastly, I don't know why anyone would do this but I clicked FL keys on the seedling markets map and it said, "Finding your location....." for a long time. Are you able to replicate? Again, it's not likely that anyone would want to plant or pull seeds from there but it just caught my attention so I thought I'd mention it.

Overall, I agree that it'd be good to get additional alpha testers to try to "break" this especially along the edges of the domain.

daviswx commented 8 years ago

Here are some answers to your questions/observations:

  1. (No data for future time slices) This should be fixed now. I just had to add a check for '9999' values, since one of the standard deviation datasets was using that, rather than 0, for missing data.
  2. (Errors for no future data) I don't have a great answer for this one. It is definitely confusing to see data displayed on the map but see a "no data" error. Do we need a special error for cases like this? Something like "Future [or Historical] data is unavailable for your selected location"?
  3. (Ranges not disappearing on an invalid click) This should be fixed. It's one of those cases I never tried in my earlier testing, so I'm glad you tested it out.
  4. (Hanging on "Finding your location") This should be fixed. I was just never telling the page what to do if no data was returned, so it appeared that it was taking forever to find your location.
hadinon commented 8 years ago

Thanks! Here are my comments:

(1) Yep, seems to be fixed. That's weird though... do you know which one std deviation dataset was doing that? They should all be the same so I'd like to investigate.

(2) Yeah, I think adding your suggested wording might be the best way to handle these situations!

(3) Yep, seems to be fixed! I just noticed that the "Climate Change Scenario" line still shows up although that isn't a huge issue. Also noticed that the ocean background layer is not showing up. Is it showing on yours or did we decide to remove this feature? image

(4) Yep, this is fixed.

daviswx commented 8 years ago
  1. It's the 1 SD below dataset that's listing missing values as 9999 instead of 0.
  2. New wording has been added. If historical or future data is missing, the no-data error will reflect that. The timeseries error will not change (it just says "Complete time series data is unavailable for your selected location.") as a matter of simplicity with updating my code.
  3. The Climate Change Scenario will now disappear if there is no data. Also, the ocean background tiles from OpenStreetMap have been giving me a "502 (Bad Gateway)" error all day. Not sure why, but hopefully it will come back or we will figure out a different place to get it.

By the way, in looking at that screenshot, I might have an idea about why the time series sometimes gets hidden beneath the bottom banner for you. It looks like your vertical resolution is greater than mine (1200 px for you vs. 979 px for me), which means my page needs a vertical scroll bar but your's doesn't. I'm guessing that might cause the time series to initially display in the remaining white space on the page, then decide it needs to scroll, which somehow causes the overlap. Anyway, I can investigate it more and see what I come up with.

hadinon commented 8 years ago

2) looks good on the dev site but when I click way out over the ocean within the extreme min temp tool, it flashes (highlighted in red) three times saying, "Projected data is unavailable for your selected location." -- are you able to replicate?

3) is fixed!

daviswx commented 8 years ago

I fixed the first bug (no-data message flashing red three times) on the dev site yesterday, so if you're seeing it there, maybe try clearing your cache. I haven't yet committed that update to the alpha and beta sites, but I will once we confirm whether the time series pop-under bug is fixed.

hadinon commented 8 years ago

OK yeah, I forgot to say that part... it was only occurring on the non-dev sites so it sounds like that's OK because it wasn't fixed on those yet. :)

Just found one more thing with the seedling deployment related to 3) above. The ranges disappear when you click over Mexico (and the ocean) but when clicking over Mexico, the legends above the maps do not disappear...

image

daviswx commented 8 years ago

This should be fixed now. I just had to dot all my i's and cross all my t's, figuratively speaking, when an invalid data point was chosen on these tools.

I will close this issue (again) but since these sort of bugs keep cropping up, I imagine it will eventually be re-opened later!

hadinon commented 8 years ago

Yep, it's fixed. :+1: Thanks!

daviswx commented 8 years ago

This issue rises again! On the Summer Temperature tool, data labels and the timeseries appear on the Projected Change map when clicking over the "protrusion" in western Canada.

daviswx commented 8 years ago

The issue with the Summer Temperature tool is fixed on the dev site. Any locations with no historical data (like the Canada protrusion) should return a no-data error.

hadinon commented 8 years ago

This is not necessarily a bug but more of an FYI... while I was testing to make sure the Canada protrusion bug was fixed, I noticed that historical avg values < 60 F for a selected location do not show up on the time series (see image below). This is obviously because the min on the y axis was purposely set to 60F because we were tailoring this tool to the lowest values within the PINEMAP domain. If we decide to expand this tool for a broader region, we will need to adjust this 60F starting point but for now, I think it's good. So, that's why it's more of an FYI for future applications... :) image

daviswx commented 8 years ago

I hate to resurrect and reopen this old issue, but I think I inadvertently found another example where clicking a point with no data does not return an error.

In this case, it seems to be happening on the Minimum Temperature Thresholds and Summer Dryness Index tools, but not the Summer Temperature and Precipitation tools. The lat/lon is 36.19°N, 75.68°W. All of the values are showing as 0.0.

image

I will investigate to see if I can determine what's causing it, then patch that problem.

daviswx commented 8 years ago

After investigating, it seems that this bug was happening whenever you clicked over the ocean, not just on a near-shore point like the example I gave might have suggested.

The problem was an unintended consequence of the fix I made a few weeks ago to limit Projected Change values to the magnitude of the historical average and Projected Average values to zero. Values over the ocean were missing (-9999) but were being set to zero by this check, and were displaying on the maps and time series as such.

I have modified my code on both the PINEMAP dev site and Climate Voyager dev site to include this fix. I will push the fix to the PINEMAP public site and close this issue in the next commit.

daviswx commented 7 years ago

This was officially fixed with the latest commit and rolled out to the public site.

hadinon commented 7 years ago

Here is an example where a value is shown yet I would expect it to return an error stating that data is unavailable at that location. I'm am 99% sure that I know why this is happening but wanted to mention it to get your thoughts. image

It's interesting because this returns the error that I would expect: image

Here's what the Projected Average display looks like (notice there are projected grid cells near Cape Hatteras but not near the left location, which is Manteo/Roanoke Island? Perhaps this has to do with the Lost Colony?! :wink:

image

daviswx commented 7 years ago

Even though your first clicked point looks like it's under water, the ocean mask is just coarser than the county boundaries, which is why areas like that are shown in blue.

You can see this a bit more easily if I turn off the ocean mask. The MACA projections include exactly one grid cell for that part of the Outer Banks, which just happens to match up with your first click location but doesn't include Roanoke Island:

image

hadinon commented 7 years ago

Ah OK, that's what I was thinking! Well if anyone asks then we can give them a good answer, right? I honestly feel that it looks so much better with the ocean mask turned on so no action items here. Just wanted to double check on that. :smile:

daviswx commented 7 years ago

Yep, I wish we could use the higher-resolution ocean mask but it takes about 10 or 15 seconds to load. So I guess we're stuck with the coarser version.