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

Do not incorrectly add "source=survey" tag to changesets #298

Closed
simonpoole opened this issue Jun 6, 2017 · 23 comments
Closed

Do not incorrectly add "source=survey" tag to changesets #298

simonpoole opened this issue Jun 6, 2017 · 23 comments
Assignees

Comments

@simonpoole
Copy link

StreetComplete is adding a fixed "source=survey" tag to all changesets it generates. The source tag is clearly for use for either human input or possibly an algorithmically determined value, adding a fixed value is clearly wrong (and yes there are people generating changesets with SC that are geographically so large that they are very unlikely to have actually been generated by survey).

@westnordost westnordost changed the title Do not add nonsense "source=survey" tag to changesets Do not incorrectly add "source=survey" tag to changesets Jun 6, 2017
@westnordost westnordost self-assigned this Jun 6, 2017
@westnordost
Copy link
Member

westnordost commented Jun 6, 2017

Absolutely, this is an issue. I did not anticipate that a mobile app which is furthermore being advertised as a surveyor app would be used for couch mapping. But apparently it is.

So, actually, I am already working on a solution and I will include this feature in the next update which is otherwise just a bugfix update.

This is how it will look like:

If the user's GPS location is near the solved quest (i.e. max 50 meters + GPS inaccuracy radius away from it), the app works as before.

If the solved quest is further away or there the GPS position is unknown, a warning dialog is shown (for every solved quest), with the following content and wording:

How did you get this information?

Note: You can only collect ★s for quests you solve on-site._
[CANCEL] [I AM THERE NOW] [VISITED BEFORE]

The I am There Button is only shown if the GPS position is unknown and when clicked, will mark the quest as solved by "survey" (so, behavior like before). The Visited Before button will mark the quest as solved by "local knowledge".

On uploading the changes, solved quests with "survey" will then appear in different changesets than solved quests with "local knowledge". For the latter, the ★ counter will not go up and it will also not appear in the leaderboards I plan to implement later this year.

I think you may guess the reason for it. It is the middle ground between completely prohibit "remote" editing and the current situation. There is the valid use case of people who visited a place before and remember things they didn't have the time (or smartphone or other reasons) to enter on-site, so they do it later in the hotel / at home but I do not want to encourage this through gamification. (Because obviously it could be misused by users who use Satellite data to add street surfaces etc. for whole towns).

@Sora1248
Copy link

Sora1248 commented Jun 8, 2017

In general I like your idea but I have a remark:
I usually have GPS deactivated as it has no value for me and it really drains my smartphone's battery. It only happened twice that I had to switch GPS on as reading the map wasn't enough to figure out were I am exactly (and one of them was due to the missing tiles bug). So showing this dialog for every quest sounds really annoying to me.

westnordost added a commit that referenced this issue Jun 8, 2017
@westnordost
Copy link
Member

Well, it's a balancing act. A less obstrusive solution would probably require a UI redesign. For now, this has to suffice.

westnordost added a commit that referenced this issue Jun 8, 2017
…uest (#298)

not completely tested yet

# Conflicts:
#	app/src/main/res/values/strings.xml
westnordost added a commit that referenced this issue Jun 10, 2017
In case user is moving fast (passenger): distance is now the minimum distance between quest geometry and line between location when the form was opened and when the user clicked "okay"

Correct distance calculation into meters. Before, it was simply wrong (calculated bigger distances the further you were from the equator).

Change layout of dialog again: Include the reason why the question is asked in the first place (=the rough measured distance to the quest) and make the note about the stars less prominent.

Concession: the app offers the "I am there option" for quests that are up to 250 away but only does not ask for less than 50 away.
@westnordost
Copy link
Member

westnordost commented Jun 10, 2017

I refined it a bit after very early feedback on the changeset comment.

See the last commit (message) to see what I changed. I'd be happy if those who gave feedback for this so far ( @Binnette, @Sora1248, @simonpoole ) would test if they find this is a good compromise before I release the next version. (I was planning to do this this weekend.)

@westnordost westnordost reopened this Jun 10, 2017
@westnordost
Copy link
Member

I wrote @Binnette wrong, not sure if the notification then works after edit.

@matkoniecz
Copy link
Member

matkoniecz commented Jun 10, 2017

I refined it a bit after very early feedback on the changeset comment.

Additional feedback - I quite often cycle and before part of trip I check what StreetComplete proposes to check along. I make edit minutes after passing given location - sometimes there is even no safe way to stop to make edit. So at least for my usecase either increasing limit to say 200, 500m or setting "I don't care about stars and I am not making edits based on guesses" that would stop this dialog from appearing (at cost of not earning stars) would be nice.

@Sora1248
Copy link

@westnordost I'm not sure if I miss something obvious (I'm way more familiar with GitLab and not GitHub) or if you expect me to compile the source myself. In the latter case: As I'm not an android developer I have neither the toolchain installed nor the experience in using it.
So if you could supply a .apk I would test it and give feedback. Otherwise I have to pass.

@westnordost
Copy link
Member

@matkoniecz It's not about stars (only for those who care) but about tagging the changeset with the correct source.

@antonv6
Copy link
Contributor

antonv6 commented Jun 10, 2017

As people have noted, being in a different place now doesn't mean that you didn't actually survey the place shortly before. I sometimes take a picture of a place instead of using the app right there, mostly for the "opening hours" quest. I get to it later and want it to be tagged as source=survey.

@westnordost
Copy link
Member

westnordost commented Jun 10, 2017

Well, you see there is a conflict of interest. A compromise must be reached. Just trusting the user that he will have surveyed the place is not an option. Otherwise, this ticket would not exist.

Originally I planned that it is only possible to solve quests on site.

@matkoniecz
Copy link
Member

matkoniecz commented Jun 10, 2017

@matkoniecz It's not about stars (only for those who care) but about tagging the changeset with the correct source.

I would prefer to drop source tag and star gain automatically rather than getting bothered by confirmation dialogs (with false positives as preferable to confirmation dialogs).

@westnordost
Copy link
Member

That is an option (though then I did all this implementation for nothing). However, this promotes edits with StreetComplete that are in fact not based on surveys. That these are already made is the reason for this ticket. StreetComplete is explicitly a survey app and should promote this.

Another implementation would be that if the user is more than X meters away from the quest, the warning dialog pops up, simply asking the user if he really surveyed the place (Yes / Cancel). The app will then continue to always tag changesets with source=survey. This doesn't get rid of this warning dialog but at least makes clear to the user that mass-tagging remote places is something that is not welcome. I.e.:

Did you check this on-site?
Only information that you verified on a survey should be entered.
[I will check] [Yes]

@westnordost
Copy link
Member

@Sora1248
I uploaded a debug build to http://westnordost.de/misc/streetcomplete-0.11+debug.apk

@westnordost
Copy link
Member

The longer I think about this simple confirmation dialog mentioned in my post above, the more I like it.

  • Implementation is simple (I can throw all that code away again)
  • User is informed that only surveyed stuff should be entered (instead of leaving it open in current version)
  • OSM contributions and the app are dependent on good faith (that the user isn't lying) anyway, so telling him that he should survey is enough
  • There could be a checkbox at the bottom "Do not show again" after the dialog showed a couple of times

@simonpoole
Copy link
Author

The problem with gamification is that it entices people to cheat and we've seen that time and time again (in this case simply copying street names from "other" sources).

Requiring the contributors to be at least near the location would be a good idea.

@westnordost
Copy link
Member

To cheat, you could always turn your location off because in the implementation that is in the repos now, the app then asks whether this is survey or local knowledge. I don't want to lock out all users who do not want to turn on their location and/or can't get a GPS fix.

The "beauty" of the method in #298 (comment) that there is no room to "cheat" because the app will always accept the edit (and give the star) but just suggest every time that this must be added from a survey.

Anyway, what would you suggest for a maximum distance? You know, there is always the use case of people doing a survey with camera and notes and then returning somewhere else and using the app to enter the data (see @antonv6 ).
(And, as I noted, this could always be avoided by simply turning location off.)

@Sora1248
Copy link

Even though you seem to be about to change it again I've tested the version you gave me anyway:
As excepted the dialog was bothersome without GPS.

In case user is moving fast (passenger): distance is now the minimum distance between quest geometry and line between location when the form was opened and when the user clicked "okay"

This hasn't worked for me: I opened the surface quest about here and clicked "okay" here. I got a message that I'm more that 150m away. The real distance between the street and the opening/closing-line is ~3m.
If you happen to fix this here's another test case: The distance between the line (0°; 0°) and (0°; 1°S) and the point (0°00'05.4''W; 51°28'40.6''N) should be somewhere around 5731km but not ~100m. This should check for one of the usual pitfalls when calculating the distance between a point and a line segment. Speaking from experience here (also I've never done it with geo coordinates).

As @antonv6 already noted: I don't always enter things right away, too. The fact that the app won't let me add this as a survey afterwards doesn't look right.

Here's a spontaneous idea that I have:
What about a simple "expert mode" that disables the cheat checking, may ask once at the start of a new mapping session if it's a survey or not and also disables the gamification aspect (so no stars are earned).
I don't think experienced mappers use StreetComplete due to the gamification concept but more due to the fact that your app is a nice way to collect information that are otherwise bothersome to collect.
That should reduce the conflicts between the need of cheat checking due to highscore chasers and mappers which just want to improve OSM. Personally I don't care about the stars. Please correct me if other experienced mappers see this differently.
But there's one catch: To really avoid cheating the mode used should be stored in the changeset so expert mode changesets won't count in the leaderboard or when the score is synced (when those are implemented). I don't know how to do it in a nice and clean way (aka without making up some undocumented tags or abusing the create_by which probably result in "discussions").

@westnordost
Copy link
Member

westnordost commented Jun 11, 2017

This hasn't worked for me: I opened the surface quest about here and clicked "okay" here.

Thanks for testing! I will have to test it myself and find the bug then. For geo calculations I use JTS to find the two closest points (between the line and the element geometry) and then calculate the distance in m between these two points. The reference system is WGS84 (same as OSM).

Regarding expert mode: I thought about later, when I really add the leaderboards etc. (I mean, not its just a counter that even gets reset if you reinstall the app! :-D) I implement that the app detects whether you are an established member of the OSM community or new, using different heuristics from HDYC. (I.e. whether you edited at least X changesets with a proper editor, how old your profile is etc.). Based on the findings, the app could be more strict towards new users but generally trust experienced ones. But this depends on the HDYC api being available which may still be some time plus I do not plan to add any gamification till milestone 2.

@Sora1248
Copy link

Ok, if I understand your description and the source code correctly you're searching for the two points of each geometry that are nearest to each other. As a result you're only considering the endpoints of the line. That won't give you the distance between the nearest point and the line. You would have to project the nearest point onto the line and measure the distance between the projected point and the nearest point of the quest object to get the correct distance.
I don't know JTS and if it has a function to do this.

Otherwise in an euclidean space this would be just a bit of vector mathematics:
l1 = First point of line
l2 = Second point of line
p = Point
· = dot-product
|...| = Norm of a vector aka it's length
s = -((l1 - p)·(l2 - l1))/(|l2 - l1|²)
Limit s to the range 0 ... 1 for line segments. That's what I meant with my test case.
p' = l1 + s*(l2 - l1) (Projected point)
d = |p' - p| (Distance)
See also: http://mathworld.wolfram.com/Point-LineDistance3-Dimensional.html
Well... LaTeX would have been nice here...

Anyway: As the earth isn't a disc on four elephants and a tortoise our coordinates aren't in an euclidean plane. But as we're only concerned about distances in a small area it's probably fine to ignore the potato-shape of the earth and use it nevertheless. But WGS84 won't work directly due to the distortion with increasing latitude.
So in pseudo-codish:

Convert points of quest geometry and line into an undistored coordinate system
For each point in the quest geometry
  calculate p' and d
Pick the point with the smallest d
Convert its point p' back to WGS84
Calculate the distance between the point and it's projection.

@Sora1248
Copy link

Well, I guess you still haven't got my point: DistanceOp.nearestPoints seems(*) to just pick one of the two endpoints of the line segment. This completely ignores the fact that we have a line segment and the nearest point can be anywhere on the line segment.

(*) I deduct this from the behavior and measured distances that I've noticed here:

This hasn't worked for me: I opened the surface quest about here and clicked "okay" here. I got a message that I'm more that 150m away. The real distance between the street and the opening/closing-line is ~3m.

Nevertheless: I've just looked a second time over the JTS documentation and finally found what I've missed yesterday: https://locationtech.github.io/jts/javadoc/org/locationtech/jts/algorithm/distance/DistanceToPoint.html
That overload with a LineSegment parameter looks like it might work.

@westnordost
Copy link
Member

I looked through the source of JTS right now and it looks like you are right. I think if JTS doesn't offer this, I won't refine this behavior though. Using the endpoints of the line segment is also fine.

DistanceToPoint doesn't help in this case, it needs to be DistanceTo( any geometry, line )

westnordost added a commit that referenced this issue Jun 14, 2017
westnordost added a commit that referenced this issue Jun 15, 2017
In case user is moving fast (passenger): distance is now the minimum distance between quest geometry and line between location when the form was opened and when the user clicked "okay"

Correct distance calculation into meters. Before, it was simply wrong (calculated bigger distances the further you were from the equator).

Change layout of dialog again: Include the reason why the question is asked in the first place (=the rough measured distance to the quest) and make the note about the stars less prominent.

Concession: the app offers the "I am there option" for quests that are up to 250 away but only does not ask for less than 50 away.

# Conflicts:
#	app/src/main/res/values/strings.xml
westnordost added a commit that referenced this issue Jun 15, 2017
…hould only enter data only based on survey (#298)

# Conflicts:
#	app/src/main/res/values/strings.xml
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

No branches or pull requests

6 participants