-
Notifications
You must be signed in to change notification settings - Fork 34
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
use labels to override trip mode #476
Comments
@st-patrick the metrics code in general is here
https://github.com/e-mission/e-mission-server/blob/8747e2279393f05f86e6be58f94de77909a4d455/emission/analysis/result/metrics/time_grouping.py#L118 And here's where we group the sections by the sensed mode. |
Let me think through the best way to modify that, for the record.
The biggest challenge with this approach is that the So a rough outline of the proposed design is:
You can then use the new objects by using the This is also consistent with the reproducible pipeline because:
That seems like a pretty solid design with no holes. @st-patrick let me know if you have additional questions |
wrt
you would change inferred -> confirmed and sensed -> final in the GeoJSON export as well Then all the data returned from the |
@st-patrick you probably want to send out a draft PR once you have a significant chunk of the code written so that I can review and give feedback. Since this is a non-trivial change, probably best to make the development/review cycle interactive. |
@shankari thanks, this will be a great feature, and thanks @st-patrick for working on it :-) The solution you describe seem fine, Another suggestion : |
You are absolutely correct the existing section segmentation and inference steps will not process the older Note that every pipeline step manages its own So to return to your use case
then the confirmation will be saved as So, before the @PatGendre Does this make sense? |
Definitely makes sense to add purpose. Not that sure about custom info because the standard algorithms can't process it without understanding its structure. Can we defer that until it is needed so that we don't overengineer? |
Yes, it's very clever, I think it will work for the proposed used too :-)
Yes |
Just for the record, I just wanted to point out a slight difference between this step and other previous steps. The previous steps are identical under replay. So every time you run the pipeline, the output of every intermediate step will be identical to the previous runs. But for the proposed new This is not really an issue - as we have seen, the design will still work. But it is a subtle little difference that we should note for interaction with subsequent design decisions. |
@jf87 fyi in case this is useful for you too 😄 |
Quite frankly I don't understand where I would even begin with these changes. Over the last days, I have tried to get the trip data with the labels at every request but couldn't find anything, since dataframe and timeseries are not serializable and made debugging quite the headache. Is there any way we could implement a serialization for that? Also, I don't think I will have time to work on the above mentioned solution, simply because we only have one week left and I just don't have the comprehension of the server code that is needed for that. But if you can draft something a little more practical and add some hints for debugging, especially how to access dataframe and timeseries data, that would be a great help. There's probably a really simple way I didn't see. I saw that in time_grouping you used .iloc[i] but since the data doesn't contain any labels at that point, it didn't really help my case. |
@st-patrick dataframes are pandas dataframes. There are tons of pandas tutorials on the internet - it is part of the standard data science toolkit. https://duckduckgo.com/?q=pandas+dataframe+tutorial&t=ffsb&ia=web should help you get started. not sure what you mean by serializable, do you mean that they don't print properly? How are you trying to print them? I don't have time to test this right now, but per stackoverflow,
Here are even more detailed steps:
This will get you a working solution in the case where the confirmation happens almost immediately. Once I review that, the second pipeline step should be much easier and will handle the other case in which the confirmation comes later. |
Since there have been requests for this from both DFKI and Heidelberg, and now I need it for the CEO e-bike project, I am going to tackle this issue now. |
@shankari that's great :-) |
Picking this up again, one challenge is that the confirmation currently happens at the trip level and not the section level. How do we then deal with creating One option is to only create But then how would we deal with This will be fixed if/when we have trip editing in place but we need to figure out what to do as a temporary workaround. |
Another challenge is that some of the data is genuinely represented at the trip level. For example, a trip has a purpose, not a section. |
@shankari I agree with you, the major difficulty may be that the mode and purpose buttons are at the trip level, so there is no clear way to induce modes at the section level. Actually the mode button should be named "principal mode for the trip" and possibly could be pre-filled with the mode totalling most of the trip length, so that we could have a relation between trip (principal) mode and section modes (thus possibly changing section modes - i.e. confirmed section mode - if the principal mode is modified) ... but that would be complicated to implement and to understand for the end user! |
@PatGendre @robfitzgerald @jf87 @asiripanich since all of you have worked with the data model, feedback would be appreciated Here's a more detailed designwe will have both
Since all our calculations are currently based on mode, if the Whoo! That was a bit complicated but I think it works for now. |
From a UI perspective, we will continue to show the However, in the dashboard and the CEO ebike gamification, we can switch to confirmed trips and confirmed sections. Concretely, for the CEO ebike gamification, we can use confirmed_trips to determine the number of ebike trips and the % of travel by ebike. For the dashboard, we can switch to getting the metrics from |
i agree with the general strategy here, to follow the pattern of building these immutable documents and not to lose any information. regarding the algorithmic selection of primary modes, a few thoughts (which both may not be helpful at this point):
longest by distance or time? also, i could imagine this might differ by survey, where users may want to inject their own "primary section function" (like, for instance, "if {driving|transit} appears anywhere, set it as primary").
might be helpful (?) to write this "analysis_mode" to a field so the user has a record of it.
🦉 |
Good point. I was going to use distance, since the primary mode is typically motorized, and likely to be much faster than the modes used for the first and last leg.
my assumption was that if people wanted to change this, they would change it in the code. But I guess I could have people pass in a primary section function instead. I might not do that in this first pass though, pending evidence that people actually need it. |
We can actually do this by adding an entry for We originally started calculating values on the client because we were also calculating the calories burned and we didn't want to send over weight and height information to the server. But the CO2/EE calculations are making more and more sense on the server. |
hi @shankari here are a few thoughts:
It is still not clear to me if the end-user will really understand what "primary section" vs "primary mode" means, and what it implies in terms of metrics calculation.
This a reasonable rule.
I agree, as long as the weight/height privacy can be managed and/or the end-user agrees to send these personal data to the server. |
I think we would split the calculations. CO2/EE would be on the server; calorie (which doesn't depend on motorized mode) would be on the phone. |
I want to clarify that this will not necessarily affect the UI at this time. The UI currently displays section modes at the top of the card, and displays trip mode overrides at the bottom. The proposed data model will allow us to continue doing that.
|
@shankari Thank you for clarifying, I did not get that point.
Also, the diary screen code could label automatically the primary mode button (with the primary section inferred mode when a section makes >50% of the length of the trip) but it might not be very useful. |
Yes, that would also be confusing, as you pointed out earlier. We already show the inferred mode at the top of the trip card, so it is not like the information is missing. We can change this later if we have time to run some user tests. |
One final consideration while creating the pipeline is how to set the timestamps for pipeline states. We will need to use the
Let's try a different approach:
|
After doing the heavy lifting for implementing e-mission/e-mission-docs#476 this is actually super simple - Switch to reading confirmed_trips instead of cleaned_trips - This leads to a table with user_inputXXX for each user input XXX - Remove the user input prefix We now end up with a table with the user inputs as columns. Et Voila! Please see screenshots in PR This doesn't fully solve the problem of how to determine whether trips have been confirmed or not, but that will require some careful design to be sufficiently general. I will open an issue for that and send a separate PR.
Fixed in e-mission/e-mission-server#780 |
Actually, that only fixed the trips, we also need to fix sections. |
While matching user inputs on the server, found that user input matching was broken for some cleaned trips on the phone. e-mission/e-mission-docs#476 (comment) Expanded the user input end check to fix. e-mission/e-mission-docs#476 (comment) Will merge into master after additional testing on the branch. + bump up the allowed delta to 15 minutes since the time threshold default for the distance filter is 10 minutes.
The enhanced trip matching (75129db) caused a regression in which a spurious trip (not a trip) that occurred after the real trip fit the criteria for a match. And since it was confirmed after the real trip, as you would expect while going down the trip list, it was matched preferentially. e-mission/e-mission-docs#476 (comment) Fixed by checking the degree over overlap and rejecting too short matches
While matching user inputs on the server, found that user input matching was broken for some cleaned trips on the phone. e-mission/e-mission-docs#476 (comment) Expanded the user input end check to fix. e-mission/e-mission-docs#476 (comment) Will merge into master after additional testing on the branch. + bump up the allowed delta to 15 minutes since the time threshold default for the distance filter is 10 minutes.
The enhanced trip matching (75129db) caused a regression in which a spurious trip (not a trip) that occurred after the real trip fit the criteria for a match. And since it was confirmed after the real trip, as you would expect while going down the trip list, it was matched preferentially. e-mission/e-mission-docs#476 (comment) Fixed by checking the degree over overlap and rejecting too short matches
* Improve the matching of user inputs to cleaned trips While matching user inputs on the server, found that user input matching was broken for some cleaned trips on the phone. e-mission/e-mission-docs#476 (comment) Expanded the user input end check to fix. e-mission/e-mission-docs#476 (comment) Will merge into master after additional testing on the branch. + bump up the allowed delta to 15 minutes since the time threshold default for the distance filter is 10 minutes. * Fix regression in enhanced trip matching The enhanced trip matching (75129db) caused a regression in which a spurious trip (not a trip) that occurred after the real trip fit the criteria for a match. And since it was confirmed after the real trip, as you would expect while going down the trip list, it was matched preferentially. e-mission/e-mission-docs#476 (comment) Fixed by checking the degree over overlap and rejecting too short matches * Fix the infinite scroll user input matching to be consistent with the refactor in 21ab3fa and 6bd731a
Hi @shankari
|
+ Fix the existing TestTripQueries for real data by passing in the correct trip_id instead of the first entry every time + this exposed several flaws, fix them by expanding the check. The corresponding UI fixes are in e-mission/e-mission-phone@75129db - Bump up the trip end buffer to 15 minutes since the time threshold default for the distance filter is 10 minutes e-mission/e-mission-docs#476 (comment) - Expand the trip end check to handle the corner case, where if the user input end is significantly after the trip end because of weird sensing issues e-mission/e-mission-docs#476 (comment) + This expansion generated a regression in which a spurious (not a trip) that occurred after the real trip fit the criteria for a match. And since it was confirmed after the real trip, as you would expect while going down the trip list, it was matched preferentially. e-mission/e-mission-docs#476 (comment) Fixed by checking the degree of overlap and rejecting too short matches. Corresponding UI Fixes are in e-mission/e-mission-phone@9438799 + Implemented a function to find the matching trip given a user input - Generalized the validity checks to take both a trip and a user input - Created two curried wrapper functions: one which took a trip and the other which took a userinput - Generalized the final_candidate function to take in the curried function and invoke it, and fix the entry detail logging to match - Create a new function that retrieves all trips within a one day window in each direction based on the user input start timestamp + Added a new test to check the new function - The test loads the data - Runs the pipeline - Loads the user inputs - Finds the trip matching each user input - Note that there can be duplicate entries for each trip as users override their prior inputs, so we may have duplicate matches, as in the case with the purpose confirm objects. Testing done: User input related tests pass
@PatGendre the deployer dashboard (emdash) uses the user labeled modes, but the in-app dashboard (the "metrics" screen) does not, at least partially because I wasn't sure how to deal with the mismatch between trip and section labeling that you outlined. The A-mission branch of the project (https://github.com/xubowenhaoren/A-Mission) from UW supports confirming sections, along with a bunch of accessibility improvements. If somebody wanted to merge the changes over to master, it would help a lot wrt modifying the in-app dashboard as well. |
@shankari thanks ! Yann will have a look at A-mission, of course I'll tell you if we envisage merging this appealing section labeling into master (we'd have to find a budget). |
@shankari FYI as there is no budget for improving to complete the labeling feature with indicators taking into account the modes labeled manually by the user, it was decided to remove the labeling feature (at least for a few months), because we think the user won't understand why the (mode, purpose) labels he enters have no effect on the dashboard indicators. And I have another question, as I've seen that GabrielKS is implementing a "label inference pipeline" : do you have a kind of "functional spec" of what this pipeline will do (on the server and on the app side)? Thanks |
@PatGendre I apologize for the delay in responding to your comments about the And I have a couple of interns working on improving the labeling by determining common and novel trips. The related issues are: This is a key component of our ongoing work since there is information that we cannot automatically detect - e.g. purpose and replaced mode. So we have to urgently reduce the user labeling burden. |
@shankari no problem, it is not an urgent question for us! FYI Fouad and Yann worked on clustering stops outside of e-mission in postgis in 2019, so as to produce statistics on frequent places and frequent trips between places, even if we didn't intend to try ML on automatic mode/purpose labeling, it was already interesting. We would like to do this again with la Rochelle, but rather in a python notebook than in postgis, but (looking at it quickly) I've found the k-means clustering methods of postgis available in shapely for python... |
We added these as part of the design in e-mission/e-mission-docs#476 (comment) However, we never filled them in ``` $ grep -r primary_section emission Binary file emission/core/wrapper/__pycache__/confirmedtrip.cpython-39.pyc matches Binary file emission/core/wrapper/__pycache__/confirmedtrip.cpython-37.pyc matches emission/core/wrapper/confirmedtrip.py: "primary_section": ecwb.WrapperBase.Access.WORM, $ grep -r inferred_primary_mode emission Binary file emission/core/wrapper/__pycache__/confirmedtrip.cpython-39.pyc matches Binary file emission/core/wrapper/__pycache__/confirmedtrip.cpython-37.pyc matches emission/core/wrapper/confirmedtrip.py: "inferred_primary_mode": ecwb.WrapperBase.Access.WORM, ``` And we are now adding a summary of all the sections into the confirmed trip, to be consistent with the error bars branch https://github.com/e-mission/e-mission-eval-private-data/pull/33/files#diff-bc613b970b833193aafe96cfd4ce0c145cbe0a4a9c52b8967b60cf936fc260b6 So we don't need the primary section and the inferred primary mode any more. The inferred primary mode is basically the mode that has the greatest proportion in the `cleaned_section_summary` or the `inferred_section_summary` (as the case may be).
We want to use the transportation mode label to actually impact the metrics shown inside the dashboard.
Since the labels are currently "decoration only", we need to implement some code that e.g. adds an endpoint that will basically do the same as the userMetrics endpoint but use the label as mode of transportation if a label has been applied to that trip.
My question being: in which file would I best start to look for a place to implement this feature?
The text was updated successfully, but these errors were encountered: