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

Result layers #38

Open
4lm opened this issue May 21, 2019 · 44 comments
Open

Result layers #38

4lm opened this issue May 21, 2019 · 44 comments
Assignees
Labels
enhancement New feature or request

Comments

@4lm
Copy link
Contributor

4lm commented May 21, 2019

There shall be layers with popups and chorolpethmaps that show results. For the work on this, I created a new branch "layers-in-results": https://github.com/rl-institut/WAM_APP_stemp_abw/tree/feature/layers-in-results

Parts needed for this (@nesnoj, following list could be useful for you):

Important: right now we use dummy data in the models, this has to change, by using data from the results!

@nesnoj, further infos I need for my work:

  • how many layers
  • naming and description of the layers
  • which charts in which layer

Something like this: #22 (comment)

@nesnoj
Copy link
Member

nesnoj commented May 21, 2019

Please find current version in result_visualization. I used random data for the Model RegMunPopResult.

@4lm, If you want to start implementation of result checking (disable layers and show a message in the layer list if no results are available yet), check out this js function where I try to obtain results and disable HCs when I get null. You may wanna use the same logic here (needs to be fired when result panel is clicked)..

I'll add a list of required charts today afternoon.

@4lm
Copy link
Contributor Author

4lm commented May 21, 2019

Hi @nesnoj,

thanks for the infos on the result_visualization branch, I already merged it with the branch I work on (feature/layers-in-results). Works fine - no conflicts!

@4lm, If you want to start implementation of result checking (disable layers and show a message in the layer list if no results are available yet), check out this js function where I try to obtain results and disable HCs when I get null. You may wanna use the same logic here (needs to be fired when result panel is clicked)..

I had a look at the function getSimulationResults() and spotted some challenges I would like to discuss with you a possible solution I came up with.

The logic of getSimulationResults() seems feasible as template for another function, in which I would like to extend it in the way, that on click:

  • it reads only that specific element
  • checks if data is null
  • and if data is null injects: "Das Szenario wurde verändert.
    Für Ergebnisse bitte Simulation starten."
  • otherwise does nothing?!

My problem: the popup charts data will come from the result models. Right now it's dummy data in the result models and in the future it shall be real data. @nesnoj, so doing nothing in the case of "data is NOT null" is the right thing to do - right?

I will now implement the first part of the new JS function, we than can discuss about the second part in the function ...

@nesnoj
Copy link
Member

nesnoj commented May 21, 2019

so doing nothing in the case of "data is NOT null" is the right thing to do - right?

Depends on what do you mean by "nothing" ;). The note above the layer list "Das Szenario wurde verändert..." should be removed and the layers be clickable.

I will now implement the first part of the new JS function, we than can discuss about the second part in the function ...

The detail view should return null if results are unavailable, it is not supposed to get clicked anyway..

@4lm
Copy link
Contributor Author

4lm commented May 21, 2019

image

@nesnoj
Copy link
Member

nesnoj commented May 21, 2019

Some more logic for the result layer list we just talked about:

Event: results panel click
Condition:

  • Results are up to date (property of Result class)

yes -> Actions:

  • hide notice in (collapsed) result panel
  • hide all result layers and set switches to off
  • reload result layers
  • make result layer's switches clickable

no -> Actions:

  • show notice in (collapsed) result panel
  • hide all result layers and set switches to off
  • make result layer's switches unclickable

Event: activation of region choropleth layer
Actions:

  • hide all result layers and set switches to off

Event: scenario change
Actions:

  • show notice in (collapsed) result panel
  • hide all result layers and set switches to off
  • make result layer's switches unclickable

We should discuss this prior to implementation to have in in line with some concepts/event listeners for other sim/result-related stuff. I.e. I would use JS events to check for results similar to getSimulationResults().

@4lm
Copy link
Contributor Author

4lm commented May 21, 2019

@nesnoj, just saw your comment, yes, let's talk on Thursday ❤️

@nesnoj
Copy link
Member

nesnoj commented May 23, 2019

Another important task:
Instead of loading the result layers on startup, they should be reloaded when result panel is clicked and the results are up-to-date (I edited the comment above accordingly).
@4lm can you take care of this one?
Let's talk later today about the event handling.

As those tasks will take some time to implement and test, I do not plan to have it finished by tomorrow.

I'm currently trying to feed the results to the model by modifying the queryset, it works but needs some tricks. I'll never use the django-geojson package again..

nesnoj added a commit that referenced this issue May 23, 2019
this allows to load result layers on demand instead at statup
@nesnoj
Copy link
Member

nesnoj commented May 23, 2019

Another important task:
Instead of loading the result layers on startup, they should be reloaded when result panel is clicked and the results are up-to-date (I edited the comment above accordingly).

I gave it a start:
@4lm You may want to make use of f22a9c8 where I split the layer list (for map context) into non-results layer_list and results layer_list_results. You could now add a 2nd function to load the result layers on demand.

Note: You can toggle the spinner visibility by firing toggleSpinnerVisibility()

@4lm
Copy link
Contributor Author

4lm commented May 23, 2019

@nesnoj, I had a closer look at f22a9c8 (concerning layer_list and results layer_list_results) and have some questions/remaks. This approach (by getting the layer data via context) would IMO only work if we trigger a reload of the window to get a server side render, because we are feeding the layer data via context. This seems not very elegant, also because this would introduce the weird behavior that the whole page would have to reload after the simulation finished.

Getting the layer data via REST endpoint(s) as an AJAX call would be much better suited for our use case IMO.

What do you think? Or am I missing something?

Edit: Yes, we get most of the layer data via JSON endpoints and AJAX calls, but the info which endoints to use comes via rendered context and some other data as well, like style and choropleth data as JSON, directly put into the JS via context render.
Edit: I count 28 template tags in the function main_map_init (map, options) in map.html we would have to change if we make everything AJAX. And maybe this work would also have side effects on the extraced JS files, like map_functions.js

@nesnoj
Copy link
Member

nesnoj commented May 23, 2019

Like you say, the context is static and is fed on page load, this is ok as the layer metadata always remain the same. But the actual dynamic data is taken from the serial view endpoint. I do not get why is it not possible to fire the AJAX call on result panel click and add the layers subsequently. No reload required, right?

I agree that we got heaps of template tags and I'd like to have it replaced (also to be able to move this stuff out of map init). But...you know..

@4lm
Copy link
Contributor Author

4lm commented May 23, 2019

Like you say, the context is static and is fed on page load, this is ok as the layer metadata always remain the same. But the actual dynamic data is taken from the serial view endpoint. I do not get why is it not possible to fire the AJAX call on result panel click and add the layers subsequently. No reload required, right?

Yes, you are right I think!

I see this working, if we create a clone of the function main_map_init in which we use layer_list_results instead of layer_list. We then call call this cloned function whenever we want to load new or updated simulation results. Maybe we could refactor the code that we use one instead of two functions, I'll have a look ...

@4lm
Copy link
Contributor Author

4lm commented May 23, 2019

Mmhh,

not as easy as I tought, because main_map_init it's doing a double Rittberger:

The template tag {% leaflet_map "mapid" callback="window.main_map_init" %} is creating the JS which then is calling main_map_init as a callback function where it also is putting the map object as param into main_map_init (map, options) (maybe also some options).

My next move now will be to see if I can obtain map and options somehow ...

Edit: found it in the window object, will see if I just can use it, stay tuned 😄

@nesnoj
Copy link
Member

nesnoj commented May 29, 2019

List of layers
For the results, we can recycle many of the region layers. First we need:

Erneuerbare Energien (EE)

  • Anteil Energie aus EE an Strombedarf
  • Gewonnene Energie aus EE
  • Gewonnene Energie aus EE je km²
  • Installierte Leistung
  • Installierte Leistung je km²
  • Anzahl Windenergieanlagen je km²

Energiebedarf

  • Strombedarf
  • Strombedarf je EinwohnerIn
  • Wärmebedarf
  • Wärmebedarf je EinwohnerIn

(list is not complete yet)

Additional feature: Show delta in layer
To allow for easy, visual comparison of status quo and actual scenario it'd be nice to have additional captions on each entity (municipality), like this

stemp_abw_result_layer_with_captions
)

After quick search this can might be done using leaflet's Tooltip or L.divIcon, e.g.:

Nice-to-have: The delta could be included in the popup like "[Gemeindename]: [Wert] (dies entspricht einer Änderung gegenüber dem status quo von XX)"

Not sure if this interferes with the existing entities/popups/tooltips, could you please check whether this is possible and affordable?

nesnoj pushed a commit that referenced this issue May 29, 2019
@4lm
Copy link
Contributor Author

4lm commented Jun 5, 2019

Hi @nesnoj,

I had a look at the endpoint results/ (JSON) and at the raw results in the session (session.simulation.results). For now I only can find aggregated results for the region, but not for each of the 20 municipalities - is that right?

To implement all the results layers (with the municipalities data) mentioned above I simply would replicate the steps done for the region layers ... in a later step we then can see how to fill them with the appropriate data (maybe we have to refactor some code for this, but in the end the structure of the code shouldn't change that much, so I think we can go this route) ...

@nesnoj
Copy link
Member

nesnoj commented Jun 5, 2019

Yepp, no progress yet, please go on with the replication as far as possible.
I will provide the data asap with the same JSON structure as before.

4lm added a commit that referenced this issue Jun 11, 2019
nesnoj added a commit that referenced this issue Jun 13, 2019
by using django's never_cache decorator for dispatch methods
nesnoj added a commit that referenced this issue Jun 13, 2019
* this view modifies GeoJSONResponse by adding a custom feature property
* uses dummy proxy model ResultLayerModel to prevent separate result layer models
* feature data (DataFrame) is obtained using Result.get_layer_results()
nesnoj added a commit that referenced this issue Jun 13, 2019
used by new result layer serial view GeoJSONResultLayerData
nesnoj added a commit that referenced this issue Jun 13, 2019
instead of passing model_name and custom_property via init it's is now taken from subclass attribute
nesnoj added a commit that referenced this issue Jun 13, 2019
…SONResultLayerData #38

as django-geojson's GeoJSONLayerView cannot be used here. Explanation: at serialization time the queryset must be a django QuerySet to have the 'id' included in the features (feature id needed for popup url). If a list of Datasets is provided (which results from adding an additional (virtual) column/property), it's not. But there's no way of having the id included in the GeoJSON without adjusting django-geojson. so
@nesnoj
Copy link
Member

nesnoj commented Jun 17, 2019

Thanks for the hint, I directly added properties in the model, via Model.objects.values('columnname') and with the help of some simple math. Works for me (at least for all input layers).

Hint: It's more efficient and idiomatic to use aggregation functions instead of loops as Django performs 1 SQL query for each dataset so in this example we have 80 (!) queries. That sounds like some load for the DB server.
Try e.g.

from django.db.models import Sum

MunData.objects.aggregate(Sum('gen_el_energy_wind')) + ...
...and so on..

PS: Still 4 queries... with Django's F() object one could even reduce it to have only one..

@4lm
Copy link
Contributor Author

4lm commented Jun 18, 2019

Thanks for the hint, you are right, 80 queries is way to much ... may plan was to first code a solution and optimize later. But your suggestion (aggregation function) is good and easy to implement, I will refactor the given code and use it form now on for new code ... if we need even more performance later on, we can look into Django's F() object in another refactor round, for now I would go with aggregation functions ...

4lm added a commit that referenced this issue Jun 18, 2019
4lm added a commit that referenced this issue Jun 18, 2019
4lm added a commit that referenced this issue Jun 18, 2019
nesnoj added a commit that referenced this issue Jun 20, 2019
nesnoj added a commit that referenced this issue Jun 24, 2019
@4lm
Copy link
Contributor Author

4lm commented Jul 16, 2019

@nesnoj. Why did you remove the deltas? #49

@nesnoj
Copy link
Member

nesnoj commented Oct 7, 2019

Currently on hold, might be implemented later, see branch feature/layers-in-results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants