-
Notifications
You must be signed in to change notification settings - Fork 22
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
Support custom label dropdowns from the dynamic config #89
Comments
Note, however, that Laos is not currently using custom labels - this is likely because the user truly added an custom mode (e.g. "motorcycle" or "rickshaw") by selecting "other" from the dropdown and then kept using it. We will not tackle that use case now, only custom labels specified by program admins and included in the dynamic config. |
suggested steps going forward
|
We are creating a dictionary with key as replaced_mode: drove_alone with values as mode_clean: "Gas car, drove alone" in mapping_dictionaries.ipynb, which is stored as dic_re = dict(zip(df_re['replaced_mode'],df_re['mode_clean'])) # bin modes. |
Close, but that is for the replaced mode, not for the mode. The very first figure, for example, does not use the replaced mode at all. |
The saved dic_re is being passed into scaffolding.load_viz_notebook_data(..., dic_re, ...). |
so you now need to read the mapping (the equivalent of |
The approach I'm undertaking is the following:
For the development mode, can we execute the generate_plots.py further calling the respective notebook (generic_metrics.ipynb)? While trying this it looked up for few modules from emission.storage, which it wouldn't be able to access. So, I understand an approach would be to take up the variables directly at the starting of the notebook, and execute. Is there an alternative way other than that for a development mode testing? |
There is a disparity in the mode_labels.csv and example-program-label-options.json referred from dev-emulator-program.nrel-op.json mapping. all_labels= ['Gas Car, drove alone', I understand the fix would be to make appropriate changes with example-program-label-options.json to match it with mode_labels.csv. |
Again, this is not very useful. What did you try to do to execute the
Your understanding is incorrect. Please read the high-level goals in the issue. It is not "support Given that the list of labels can be custom, we can no longer hardcode the list of colors. I don't see why the list of colors needs to be hardcoded in the first place given that we are mapping it to Note that the list of modes now also specifies the basemode. As a stretch goal, you can use the number of modes of each base mode to get shades of the basemode color and be consistent with the phone UI. But that is a future enhancement after we get the basic custom mode display correct. |
I was validating my changes with reference to the available example for "support custom labels for each program/study" which is |
@iantei you cannot update Please read through my comments on "hardcoded list of colors" carefully. |
//Testing done so far: [DRAFT: Still updating] A. Change the mode_confirm "walk" to "moped":Showcasing existing entries in the mongodb:
Results:
Executed the below script to make changes on the mongoDb:
Result of above execution:
B. Re-started the docker compose and executed the following:Executed the following:
Re-evaluated the status of mongodb:
C. Execute generic_metrics with moped data, and refresh localhost page: [Still showing old data] - The issue observed has been fixed below.Executed the following:
Results:
Refreshing localhost, still gives the below chart: As you can see in the above chart, the latest pie_chart depicting "Moped" mode is not being updated. The timestamp doesn't match with the one from the log. D. The chart is updated - refreshed the page after ~30 mins Fix: Let the notebook execution complete till present time.I travelled for half an hour or so, now it's been updated to show the below. Refreshed the page and now it's loading properly with "Moped" data as Others Execution error with generic_metrics_sensed notebookExecution: ``` (emission) root@61584fb3620f:/usr/src/app/saved-notebooks# PYTHONPATH=.. python bin/generate_plots.py generic_metrics_sensed.ipynb default ``` Result:
Execution of generic_timeseriesExecuted the following: ``` (emission) root@61584fb3620f:/usr/src/app/saved-notebooks# PYTHONPATH=.. python bin/generate_plots.py generic_timeseries.ipynb default ``` Result:
Executed Mode Specific MetricsExecuted the following command: ``` (emission) root@61584fb3620f:/usr/src/app/saved-notebooks# PYTHONPATH=.. python bin/generate_plots.py mode_specific_metrics.ipynb defaul ``` Results:
Execution for mode_specific_timeseries notebook [Error]
Executed the following code:
Result:
Executed energy_calculations notebookExecuted the following: ``` (emission) root@61584fb3620f:/usr/src/app/saved-notebooks# PYTHONPATH=.. python bin/generate_plots.py energy_calculations.ipynb default ``` Result:
|
wrt
this seems to be a dup of #93 (comment) If you can check the database and verify that you see the same
That is kind of bizarre. I don't see any error while running this
When the notebook is run with
Once you have figured it out, of course, change |
The chart not being updated is not an issue. I tried to refresh the page prior to the successful completion of the notebook script. Upon the completion of notebook script, things are working fine. |
/> When the notebook is run with
The notebook execution is not failing. So, I don't think it's required to re-execute the notebook. I will skip it, and proceeding with the generic_metrics_sensed issue. Steps followed:
This seems different. |
@iantei So the issue that you are highlighting is that reloading does not actually reload the metrics, but opening a new page does? Couple of high level comments:
|
@iantei not just going forward. There is no evidence here that the miles graph is updated, even with the new browser tab (right image) I would like to see a record of the miles_mode_confirm (and the other metrics) images with recent timestamps as well |
Here's the command I ran along with the output.
|
@iantei Testing of the non-sensed notebooks seems to be fine. However, I do not see the reason for the notebook with the sensed metrics to fail. Once we have established that reason, I am happy to merge. |
Proceeding with the generic_metrics_sensed issue. Steps followed:
Result:
This seems different. |
There're three notable issues with this change right now.
|
Where is the patch showing the statement you added? This does not look like python code to me
Also, it is not clear that |
wrt #89 (comment)
|
Obviously, the difference is due to the different mappings. But what is the difference and why does it cause the values to be different? We have to understand it and make sure that it is not an error. When we had the "other" trips completely dropped from (3), it was also due to a difference in the mapping, but it was an error. most of the values here are the same - the only difference is in "Gas Car, with others" and "No Travel". Please investigate the cause of the difference between them. |
@iantei for #89 (comment), which dynamic config did you use, and how did you use it? |
The dynamic config mentioned on #91 (comment) |
This was highlighted while creating the issue:
The fact that the current examples are similar to the default values may be confusing you. You have to understand the underlying concepts to make sure that you build a solution that works properly. As a concrete example, the CA e-bike rebate program is planning to support the following modes. Keep that in mind while designing and testing your solution.
|
In the current implementation, I am using The reason behind No 'No Travel' is because there is a different in translations.en block between Code to showcase the mapping```
|
Yes, I am still using energy_intensity.csv as the mapping which needs to be fixed. |
At a high level, you are correct that energy is more complicated since we don't compute it on the phone (currently) and so it is not part of the dynamic config. As a first step, I would suggest two things to get this change done before we expand with energy etc
What if the dynamic labels are incorrect?
For context on how we will/can expand this going forward:
|
We use some computation values present on energy_intensity.csv to compute the CO2 emission. In brief, we compute the But when we have dynamic label, we would like to make use of While using this |
In the generic Timeseries notebook, we're displaying emission charts which visualized the units as lb, miles. Would we like to make alignment to these metrics to be displayed in kg, km units. We're computing in kg, km units for cases when dynamic labels are available. |
We're mapping mode_confirm to Mode_confirm using dynamic label 's translations.en mapping.
I do see only the following listing present on the MODE in example
There's a few notable values like Should not |
Good catch. Yes, |
We're using the We're retrieving the user labeling from user_label - from For the future gatekeeping, we can provide the user with a list of option to select from [like dropdown menu]. But if we're providing custom label options, it'd be difficult to check whether the Likewise for the dynamic_config, re-designing the mapping between different labels by not using |
Yes, in the default mapping. That is the desired behavior. But not in the dynamic mapping by default because it is not split between mode and purpose.
I don't understand what you mean by this.
I don't know what you mean by this - we already do provide the user with a list of options to select from
I am not sure what you mean by this |
My understanding is We are fetching all this information related to confirmed trip from
Categorizing labels, which are not present on dic_re, but are actual
If we already provide the user with a list of options to select from, how did the mislabeling happened? What happens if there is wrong labels available on |
Because they selected "other" and typed in whatever they wanted
Why? We have a list of valid mode labels. They entered something else. That is an "other" label.
No. We cannot and will not guarantee that there is no mislabeling. Any mislabeled trips will just go into OTHER. That is the expected output. |
For the dynamic mapping:
I would like to propose the following: This way, we would not have wrong categories for different mode and purpose. |
Testing done on generic_timeseries notebook. I tested this with default mapping, prior to my changes and observed this behavior. We're showing all the labels associated with Confirmed Mode on the Legends' indices, while only showing 5 distinct modes on the timeseries graph. |
I have used kg and kms in computation of emission and energy computation instead of using lbs and miles (which was used previously), which resulted into changing all the displayed values for chart belonging to |
Sure. But it is not clear why "Not a Trip" and "Other" are showing up in the legend in the first place.
so figuring out the reason is likely to need matplotlib skills. It is not wrong (both Not a trip and Other are zero), so I would file a new issue and clean it up later rather than holding up this change further.
I think we have discussed this already. The long-term solution is to use the |
Changes have been pushed to production |
In a recent phone change, we started supporting a custom set of labels in the dropdown.
The custom set of labels is specified as a separate file linked from the dynamic config, and includes the kgCO2/km for each mode.
e-mission/e-mission-docs#945
However, this change has not yet been implemented in the public dashboard. The public dashboard still reads the user-specific mappings and the CO2 and energy equivalents from files hardcoded in this repo (notably in
viz_scripts/auxiliary_files
).This means that the custom labels will be mapped to Other, which in turn, means that the public dashboard becomes less meaningful. This is likely to be particularly challenging in international contexts - e.g.
https://usaid-laos-ev-openpath.nrel.gov/public/
The public dashboard should read the values from the dynamic config as well if they are present, and fall back to defaults if they are not.
Note that depending on how we convert between energy and emissions, we may have to have the energy equivalent stored in the trip_confirm lists as well. You can submit a PR (similar to e-mission/nrel-openpath-deploy-configs#32) to add them if required.
@ananta-nrel can you please handle this?
The text was updated successfully, but these errors were encountered: