Improving device-data-recent about air-quality-web HOT 31 CLOSED

I will rewrite this function with pi()/2 as a variable. It should , i believe, be declared as deterministic.
I'm playing golf in the rain again tomorrow. Might have time to post the rewrite for your perusal before I leave. Will then look to implement it when i get back.

from air-quality-web.

If you need a test location for a double check, use 53.750, -0.395
It the place I used as it near the middle of a lot of sensors (Derringham Bank roundabout/Calvert Lane iirc).
And you get both sensors above and some to either side, which is were the problem came from.

For reference, Old order they came out at and the distance it should be,.
32 haversine 1997.86 m 36 haversine 1886.59 m 34 haversine 3823.82 m 35 haversine 2847.05 m 10 haversine 4206.41 m

from air-quality-web.

Some tests. First using the online calculator above

Now using the ST_DISTANCE_FORMULA you gave me - big difference

Now reversing the lat/lon values

Conclusion - the ST_X and ST_Y values in your formula need reversing because we elected to store coords in the persistent field as lat-lon not lon-lat.

I will change the formula and retatest it later today.

from air-quality-web.

@sbrl - I'll look into the execute permissions thing tomorrow.

from air-quality-web.

Noted. Will look into setting this up.

from air-quality-web.

from air-quality-web.

Thanks, @BNNorman!

@bsimmo: Good idea - we can pre-calculate some bits here to reduce some computational load. I'd be curious to know how much that'll actually help though, because surely the SQL query optimiser can figure that out?

Probably best to check?

from air-quality-web.

Does this float your boat?

DELIMETER //
CREATE FUNCTION `ST_DISTANCE_SPHERE`(`pt1` POINT, `pt2` POINT) RETURNS decimal(10,2) DETERMINISTIC
BEGIN
	DECLARE rad180 double;
	DECLARE rad360 double;

	SET rad180=pi()/180;
	SET rad360=rad180/2;

	return 12742000 * ASIN(SQRT(POWER(SIN((ST_Y(pt2) - ST_Y(pt1)) * rad360),2) + COS(ST_Y(pt1) * rad180) * COS(ST_Y(pt2) * rad180) * POWER(SIN((ST_X(pt2) - ST_X(pt1)) * rad360),2)));
END
//
DELIMETER ;

I have done this on the Pi and selected #37 as the current location - one of mine - you'll see #22 #24 and #26 are very close - because they are also mine.

It throws 13 data truncation warnings...do we care? Would it be better to return a double datatype?

If this gives you a warm glow I'll implement it on the server - this morning if you can respond before 8:30 otherwise when I get back from being drenched on a golf course (damn these pre-paid competitions)

from air-quality-web.

Hey, thanks @BNNorman! The only thing I'd be concerned about is another performance issue because of the warnings, but looking at the query time there I'm not sure that we do particularly care about such a warning (though it would be nice to squash it - especially since that will save the log files from filling up).

It looks fine to me - with or without returning a double. Thanks @BNNorman!

from air-quality-web.

Would there be a problem returning a double? Otherwise could wrap a truncate() around the returned value.

from air-quality-web.

No problem at all, @BNNorman! So long as I can use it in an ORDER BY (which I'm pretty sure you can with a DOUBLE, it shouldn't matter.

I think returning a double probably the preferable option.

from air-quality-web.

Well this is what I get using Bens coords

Wondering if the lat/lon are the correct way around as per:-

https://stackoverflow.com/questions/45216121/why-is-this-python-haversine-formula-producing-incorrect-answers

This isn't my field so experts please advise.

from air-quality-web.

Hrm. Not sure. That's rather odd that it works for your location but gives strange outputs for @bsimmo's - though we do only have ~2-3dp for those co-ordinates - maybe that's the issue?

I think you've got the ordering right - it should be (latitude, longitude).

Are those warnings still the truncation ones?

from air-quality-web.

Warnings, yes. Double would have more dp. What i need is a pair or coords with a known distance between to prove the formula. Where did you get the original formula? The stack exchange solution uses atan, yours uses asin

Also here:- https://www.movable-type.co.uk/scripts/latlong.html

from air-quality-web.

For reference, mine comes from Python calculations, they are on the Forum.
One is the standard Haversine formula
These give the same as the Vincenty (using the result from the database that made me notice the problem) and also the more accurate again Karney formula (last one using a well worked on python module geopy).
pip install geopy

In case you need it, this should get the list from the database via the api and compare to others.

import json
from urllib import request, parse
from math import radians, sin, cos, asin, sqrt
from geopy import distance

api_url = "https://aq.connectedhumber.org/api.php?action="

no_of_locations = 3
my_location = ( 53.750, -0.395 )

def get_nearest_sensors(my_location, no_of_locations):
    url = str(api_url) + "list-devices-near&count=" + str(no_of_locations)
    data = parse.urlencode({'latitude': my_location[0], 'longitude': my_location[1]}).encode()
    req = request.Request(url, data=data)
    response = request.urlopen(req)
    return response

def haversine(my_location , sensor_location ):
    lon1, lat1, lon2, lat2 = map(radians, [my_location[1], my_location[0], sensor_location[1], sensor_location[0]])
    dlon = lon2 - lon1
    dlat = lat2 - lat1
    a = sin(dlat / 2) ** 2 + cos(lat1) * cos(lat2) * sin(dlon / 2) ** 2
    return 2000 * 6371 * asin(sqrt(a))
    

for x in range(0,6):
    nearest_sensors = json.loads(get_nearest_sensors(my_location, x).read().decode('utf-8'))

    # print(nearest_sensors)

    for line in nearest_sensors:
        distance_away = round(line['distance_actual'],2)
        cdistance = round(line['distance_calc'],4)
        hdistance = round(haversine(my_location, ( line['latitude'],line['longitude'])),2)
        geodistance = distance.distance(my_location, ( line['latitude'], line['longitude']))
        
        print( line['id'], line['name'], "is", distance_away, "miles away", "cal dist = ",line['distance_calc'], " -- haversine", hdistance, "--geodesic", geodistance)# nearest_sensor_check = ('id','name', 10000.0)
        #print( line['id'], "haversine", hdistance, "m")
    print("----------")

from air-quality-web.

Thanks Ben.
I have created the stored function SBRL asked for - just swapped the ST_X,ST_Y and it delivers the correct value as per my earlier posting.

delimiter //
CREATE FUNCTION `ST_DISTANCE_SPHERE`(`pt1` POINT, `pt2` POINT) RETURNS double DETERMINISTIC
BEGIN
	DECLARE rad180 double;
	DECLARE rad360 double;

	SET rad180=pi()/180;
	SET rad360=rad180/2;

	return 12742000 * ASIN(SQRT(POWER(SIN((ST_X(pt2) - ST_X(pt1)) * rad360),2) + COS(ST_X(pt1) * rad180) * COS(ST_X(pt2) * rad180) * POWER(SIN((ST_Y(pt2) - ST_Y(pt1)) * rad360),2)));
END
//
delimiter ;

from air-quality-web.

from air-quality-web.

Not sure what you are referring to there Ben - what/where is this distance_actual?

from air-quality-web.

from air-quality-web.

I see. Yes he'll be using something in SQL (ST_DISTANCE()??) but now I have uploaded ST_DISTANCE_SPHERE as a stored function he can use that.

from air-quality-web.

Hey, thanks @BNNorman! When I have a moment I'll implement that change.

Yeah, if there's a suitably small distance between distance_actual (calculated in PHP with a library) and distance_calc (calculated via SQL), I'll remove distance_actual.

from air-quality-web.

Just looked into it, and it would work.... except that my MariaDB user account (that's also used for the web interface) doesn't have permission:

PDOException: SQLSTATE[42000]: Syntax error or access violation: 1370 execute command denied to user 'sbrl'@'%' for routine 'aq_db.ST_DISTANCE_SPHERE'

I've committed the update in e86b0b4, so it'll start working on the dev branch as soon as the permissions issue is fixed.

This time I'm going to roll up a bunch of features over ~1-2 weeks before pushing them all out at once - with a beta testing period before release should beta.aq.connectedhumber.org be a thing.

@BNNorman

from air-quality-web.

from air-quality-web.

Original code was 2dp - that's 10m

from air-quality-web.

Ok added execute permission for you. Please try it.

from air-quality-web.

Ah, ok. Sounds like we'll be able to ditch distance_actual after all! I'll probably rename distance_calc to just simply distance then.

@BNNorman: Thanks so much! I'll try it when I get a moment.

from air-quality-web.

Looks like it works fabulously, @BNNorman 😺

In the interests of stability though, I'm not going to immediately release this. As I've mentioned before, I'll roll up a few other changes, have a beta testing period, and then release to stable.

Releases won't be regular (i.e. on a timed basis), but I do hope to have a steady stream of updates coming in batches.

from air-quality-web.

Any timing improvement?

from air-quality-web.

Not sure there's a noticeable difference between ST_DISTANCE() and ST_DISTANCE_SPHERE() at the moment.

from air-quality-web.

ST_DISTANCE() returns the angle between the two points so you'd need to factor in the rad/dia of the Earth which might add more micros.

But ST_DISTANCE_SPHERE() is more correct.

from air-quality-web.

Right. At the moment it shouldn't be an issue. If there's a performance issue later, I'll tackle it then.

I'm closing this issue for now since we've implemented the fix. Feel free to open a new issue if problems continue to occur.

from air-quality-web.