HSLdevcom / mal-effect-calculations

Impact assessment scripts for the Helsinki region MAL planning process
https://www.hsl.fi/hsl/mal
European Union Public License 1.2
1 stars 0 forks source link

Distributional impacts #3

Closed attesn closed 3 years ago

attesn commented 3 years ago

This PR contains calculations from MAL 2023 distributional impacts project. Scripts use Helmet-agent model results, combine data from different scenarios and visualize results. Currently interaction with Helmet agent model is done so that model prints out data for each agent and aggregations are done here.

Running scripts

Scripts are run from run.R. Some packages (sf, tidyverse, here, config) are added to renv lockfile.

Example of Helmet-results data (agents.txt) is stored in HSL Sharepoint folder (Saavutettavuuden jakautuminen). Data-folder should include helmet_zones.shp and helmet_zones_map.shp, which is used for map results. These files are found in HSL Sharepoint project folder.

Structural issues

attesn commented 3 years ago

This is still work in progress, but it might be good to do check that overall development is in line with other Pull Requests.

johpiip commented 3 years ago

@atte-wsp I left a couple more review comments now that I also reviewed the plotting functions. They are not all mandatory if your time is running low, just do what you can! 😄

attesn commented 3 years ago

Thanks for comments @johpiip ! Good that you said about breaking Pull Request into smaller pieces, I didn't think about that when creating this. Probably should have put at least plot scripts to separate PR.

I'll remember this next time and further commits into new branches. Although, I'll update above comments here.

attesn commented 3 years ago
  • In the future, please break your changelist into smaller pieces - reviewing 2200 lines of code in one PR is super difficult! 😄 For example, if this PR already works, we could merge this and you could do new branches for updates? (Edit: or another idea: you could do new branches and create pull requests to this branch instead of main. That way nothing goes to main before everything is "ready" but I do not need to review this whole PR again and again.) What do you think?

Maybe I would prefer the first option. That would make it easier to see if for example basemaps could be integrated to plotting used here.

attesn commented 3 years ago

Yep, ready for review!