INRETS and bpr2 VD functions #273

Art-Ev · 2021-10-20T12:58:04Z

Just adding INRETS and a BPR2 VD function.
Not fully tested but I think it should work almost as it is.

Art-Ev · 2021-10-20T13:00:29Z

And a quick excel to compare all AequilibraE VD functions and see the impact of the function parameters :
VD_functions.xlsx

First implementation of the French INRETS Volume Delay function. Beta is not used but kept for better consistancy across VD functions.

First implementation of BPR2 Volume Delay function. This doubles beta above capacity so that fewer vehicles are affected when capacity is exceeded. Double Beta insteed of using a Beta' allow to use only 2 parameters as for other VD functions. Integration into QGIS GUI is also easier 😜

pedrocamargo · 2021-10-20T23:22:18Z

@Art-Ev , thanks for the contribution!!!! Two things that are necessary before we can merge it into master:

Fixing style issues (installing and running flake8, a python package) on your system would point to all issues locally
Tests (all new code requires tests)

@janzill , may I ask you to check the math in the functions?

Art-Ev · 2021-10-21T09:56:03Z

First styling revision, do we need to correct all E501 error (line too long) ?
Anything to check for pyx and rst files ?

pedrocamargo · 2021-10-21T10:15:15Z

Don't worry about the failed documentation test, @Art-Ev . It built successfully but failed on uploading to AWS (obviously, as the PR comes from a fort of the original repo)

pedrocamargo · 2021-10-21T10:16:04Z

aequilibrae/paths/AoN.pyx

@@ -21,7 +21,9 @@ from libcpp cimport bool
 # include 'parameters.pxi'
 include 'basic_path_finding.pyx'
 include 'bpr.pyx'
+include 'bpr2.pyx'


Nice having in separate files and following the current pattern/organization!

pedrocamargo · 2021-10-21T10:17:19Z

aequilibrae/paths/bpr2.pyx

+
+  for i in prange(l, nogil=True, num_threads=cores):
+      if link_flows[i] > 0:
+        if link_flows[i] > capacity[i]:


This indentation here looks funny. Can you change it for the PEP8 standard of 4 spaces?

Was based on previous bpr.pyx and conical.pyx without really trying to understand why indentation was switching between 2 and 4 spaces (assuming it was related to cython ^^'). I will update

pedrocamargo · 2021-10-21T10:21:37Z

Some documentation on INRETS, @janzill http://transportproblems.polsl.pl/pl/Archiwum/2017/zeszyt1/2017t12z1_04.pdf

pedrocamargo · 2021-10-21T10:22:22Z

aequilibrae/paths/inrets.pyx

+
+  for i in prange(l, nogil=True, num_threads=cores):
+      if link_flows[i] > 0:
+        if link_flows[i] > capacity[i]:


pedrocamargo

@Art-Ev , it looks excellent, aside from the issue with indentation and the lack of testing. You can look at this file for reference: https://github.com/AequilibraE/aequilibrae/blob/master/tests/aequilibrae/paths/test_conical.py

Correction of style issues base on flake8 for .py files (E501, line too long , is ignore). Do we need to correct all E501 ? Anything to check for pyx and rst files ?

janzill

looks good to me except for style. A great improvement would be having tests for this as @pedrocamargo mentioned, but we can look into that if you don't have time.

pedrocamargo · 2021-10-21T11:21:37Z

I second @janzill 's comment on tests. We are actually expanding the guidelines on collaboration to make it easier, @Art-Ev !

pedrocamargo · 2021-10-21T12:12:22Z

I would love some comments on the new contribution guidelines.

https://aequilibrae.com/python/develop/softwaredevelopment.html

Art-Ev · 2021-10-21T12:43:54Z

looks good to me except for style. A great improvement would be having tests for this as @pedrocamargo mentioned, but we can look into that if you don't have time.

Will take a look at all of that, I'll keep you informed!

Art-Ev · 2021-10-21T14:09:06Z

Can somebody check my inrets values ? Calculated them with excel but cannot retrieve exactly the same values for Spiess' conical through excel than the values in test_conical (ok at 2 decimals but values differs beyond that)

janzill · 2021-10-21T23:16:02Z

@Art-Ev could you please change the target branch from master to develop? @pedrocamargo just added a new workflow so that collaborations and contributions are easier.

pedrocamargo · 2021-11-08T09:09:54Z

@Art-Ev , we can go ahead and merge if you change the target from Master to Develop

Art-Ev · 2021-11-08T10:22:36Z

@Art-Ev , we can go ahead and merge if you change the target from Master to Develop

I thought I did it 14 days ago after Janzill message... github tells me "Art-Ev wants to merge 4 commits into AequilibraE:develop from Art-Ev:INRETS-and-BPR2-DV-functions". Where can I correct this?

janzill · 2021-11-10T00:22:39Z

@Art-Ev , we can go ahead and merge if you change the target from Master to Develop

I thought I did it 14 days ago after Janzill message... github tells me "Art-Ev wants to merge 4 commits into AequilibraE:develop from Art-Ev:INRETS-and-BPR2-DV-functions". Where can I correct this?

I can see the change @pedrocamargo, will need to look into @Art-Ev's question regarding inrets comparison values

pedrocamargo · 2021-11-15T01:39:34Z

@Art-Ev , something went haywire with those indentations.

* Updates shapely deprecated methods (#281) * Improves performance of triggers (#278) * documentation correction (#282) * French INRETS and bpr2 VD functions (#273) * Fixes styles to expose test failures * Adds the obvious field vot to the modes table and creates and adds a script for compiling that may be useful during development * Closes issue #289 and adds small feature * fixes traffic assignment tests (#293) * Follows the Wikimedia user-agent policy (#295) * Adresses issue #285 (#297) * Allows for skims to be set after the creation of traffic classes * Improvement on skim saving * Fixes creation of skim matrices on multi-class assignment * Threading * Adds connection refresh capability * Stores binaries with all tests * Makes feather optional on import * Removes repeated code Co-authored-by: Danilo Ebbinghaus <91709021+daniloebbinghaus@users.noreply.github.com> Co-authored-by: Arthur Evrard <83211842+Art-Ev@users.noreply.github.com> Co-authored-by: Jan Zill <jan.zill@veitchlister.com.au> Co-authored-by: Jan <jan.christoph.zill@gmail.com>

@pedrocamargo

New features: French INRETS and bpr2 VD functions (#273) Adds the obvious field vot (Value-of-Time) to the modes table and creates and adds a script for compiling that may be useful during development Bug fixes/Improvements: Allows for skims to be set after the creation of traffic classes Improvement on skim saving Fixes creation of skim matrices on multi-class assignment Makes feather optional on import Documentation: Adds new instructions for contribution by @pedrocamargo in #275 documentation correction (#282) Software improvements Updates shapely deprecated methods (#281) Improves performance of triggers (#278) Workflow test by @pedrocamargo in #276 fixes traffic assignment tests (#293) Follows the Wikimedia user-agent policy (#295) Co-authored-by: Danilo Ebbinghaus <91709021+daniloebbinghaus@users.noreply.github.com> Co-authored-by: Arthur Evrard <83211842+Art-Ev@users.noreply.github.com> Co-authored-by: Jan Zill <jan.zill@veitchlister.com.au> Co-authored-by: Jan <jan.christoph.zill@gmail.com>

* Updates shapely deprecated methods (#281) * Improves performance of triggers (#278) * documentation correction (#282) * INRETS and bpr2 VD functions (#273) * French INRETS VD function First implementation of the French INRETS Volume Delay function. Beta is not used but kept for better consistancy across VD functions. * BPR2 implementation First implementation of BPR2 Volume Delay function. This doubles beta above capacity so that fewer vehicles are affected when capacity is exceeded. Double Beta insteed of using a Beta' allow to use only 2 parameters as for other VD functions. Integration into QGIS GUI is also easier * Two small changes around the edges (#288) * Closes issue #289 and adds small feature * Fixes long-standing formatting issue * fixes traffic assignment tests (#293) * Follows the Wikimedia user-agent policy (#295) * Makes sure we follow the Wikimedia user agent policy https://meta.wikimedia.org/wiki/User-Agent_policy * Adresses issue #285 (#297) * Allows for skims to be set after the creation of traffic classes * Improvement on skim saving * Updates example with change in skim saving method * Fixes typo * Fixes creation of skim matrices on multi-class assignment * Fixes final skim for travel time and introduces basic testing * Fixes example with new skimming standards * cleans tests * Relocates and Fixes path saving tests * Updates tests * QGIS adjustments (#280) * Threading * Adds connection refresh capability * Stores binaries with all tests * Makes feather optional on import * Removes repeated code * Bumps up version * Adds doc version to index * Bumps version for new release * closes issue #303 * closes issue #303 (#304) * Fixes issue 307 * Improves database structure (#310) * Improves database structure * Improves database structure * numpy 1.22 is still not the default * CI workflow to generate binaries for different NumPy versions * Sets up better tests * Sets up better tests * numpy 1.22 is still not the default * Cleans test * Run black on all source files * Adding conda_forge_recipe * Support for multiple projects (#323) * Matrix from list (#325) * Pedro/new test fix (#322) * Speeding up OSM network import * only run actions that require secrets from the Aequilbrae account. (#329) * Support logging for multiple projects, log most warnings instead of raising them (#327) * one logger per project, log most warnings instead of raising them * Multiple changes * renamed starts_logging and combine with log.py * moved additional setup code to __init__.py * undo falsely blackened line * process feedback * cleanup ci setup for unittests - do errors still occur? * Small changes to the Delaunay line creation in trying to identify consistent breaking point Co-authored-by: Danilo Ebbinghaus <91709021+daniloebbinghaus@users.noreply.github.com> Co-authored-by: Arthur Evrard <83211842+Art-Ev@users.noreply.github.com> Co-authored-by: Jan Zill <jan.zill@veitchlister.com.au> Co-authored-by: Jan <jan.christoph.zill@gmail.com> Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> Co-authored-by: PelleK <elfjes@users.noreply.github.com> Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov>

@janzill

* Updates shapely deprecated methods (#281) * Improves performance of triggers (#278) * documentation correction (#282) * INRETS and bpr2 VD functions (#273) * French INRETS VD function First implementation of the French INRETS Volume Delay function. Beta is not used but kept for better consistancy across VD functions. * BPR2 implementation First implementation of BPR2 Volume Delay function. This doubles beta above capacity so that fewer vehicles are affected when capacity is exceeded. Double Beta instead of using a Beta' allow to use only 2 parameters as for other VD functions. Integration into QGIS GUI is also easier * Fixes styles to expose test failures * Fixes styles to expose test failures * reverts white space changes in bpr * Fixes inrets tests * Two small changes around the edges (#288) * Adds the obvious field vot to the modes table and creates and adds a script for compiling that may be useful during development * Update tests/compile.py Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> * Fixes import * Closes issue #289 and adds small feature * Fixes long-standing formatting issue * fixes traffic assignment tests (#293) Brilliant work, @janzill That original test was really out of whack!!! * Follows the Wikimedia user-agent policy (#295) * Makes sure we follow the Wikimedia user agent policy https://meta.wikimedia.org/wiki/User-Agent_policy * Adresses issue #285 (#297) * Allows for skims to be set after the creation of traffic classes * Improvement on skim saving * Updates example with change in skim saving method * Fixes typo * Fixes creation of skim matrices on multi-class assignment * Fixes final skim for travel time and introduces basic testing * Fixes example with new skimming standards * cleans tests * Relocates and Fixes path saving tests * Updates tests * QGIS adjustments (#280) * Threading * Adds connection refresh capability * Stores binaries with all tests * Makes feather optional on import * Removes repeated code * Bumps up version * Adds doc version to index * Bumps version for new release * closes issue #303 * closes issue #303 (#304) * Fixes issue 307 * Improves database structure (#310) * Improves database structure * Improves database structure * numpy 1.22 is still not the default * CI workflow to generate binaries for different NumPy versions * Sets up better tests * Sets up better tests * numpy 1.22 is still not the default * use temp dir * guessing this is a case problem * guessing this is a case problem * guessing this is a case problem * it was a case issue * Cleans test * Run black on all source files * Update build_linux.yml * Adding conda_forge_recipe * Support for multiple projects (#323) * Matrix from list (#325) * Pedro/new test fix (#322) * Fixes linux building * Fixes linux building * Speeding up OSM network import * only run actions that require secrets from the Aequilbrae account. (#329) * Support logging for multiple projects, log most warnings instead of raising them (#327) * one logger per project, log most warnings instead of raising them * Multiple changes * renamed starts_logging and combine with log.py * moved additional setup code to __init__.py * undo falsely blackened line * process feedback * cleanup ci setup for unittests - do errors still occur? * Edit the correct ci file * we do need more requirements * run in docker to improve isolation * apt get update Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> * Small changes to the Delaunay line creation in trying to identify consistent breaking point * Fix some warnings Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> * Refactor tests to pytest (#331) * Refactor tests * Cleanup and some comments * fix tests * process feedback * Update tests/aequilibrae/conftest.py Co-authored-by: Jamie Cook <jimi.cook@gmail.com> * rescope session-scoped fixtures inside a class to class scope to avoid confusion * fix trailing whitespace Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> Co-authored-by: Jamie Cook <jimi.cook@gmail.com> * change the check for secrets from repo_owner to has_secret (#333) * better deal with np.memmap open files (#335) Co-authored-by: Pelle <pellekoster@octagonal.nl> * Adding capability of importing and exporting networks in GMNS format (#330) * Pedro/fixes numpy changes (#342) * Fixes nan to num * Chipping along transit structure * Fixes tests * Uses new version of Numpy for Windows 3.10 only * address issues from review * unneeded Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> Co-authored-by: Jamie Cook <jimi.cook@gmail.com> * Updates actions (#341) * Updates actions * Chipping along transit structure Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> * Fixes issue #344 (#345) * Fixes issue #344 Co-authored-by: François P <35044397+djfrancesco@users.noreply.github.com> * Bumps up version * Bumps up version (#346) Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> * bug fix (#347) * Updates actions (#349) Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> * Update Develop * stops using Kumi endpoint Co-authored-by: Danilo Ebbinghaus <91709021+daniloebbinghaus@users.noreply.github.com> Co-authored-by: Arthur Evrard <83211842+Art-Ev@users.noreply.github.com> Co-authored-by: Jan Zill <jan.zill@veitchlister.com.au> Co-authored-by: Jan <jan.christoph.zill@gmail.com> Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> Co-authored-by: PelleK <elfjes@users.noreply.github.com> Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> Co-authored-by: Jamie Cook <jimi.cook@gmail.com> Co-authored-by: Pelle <pellekoster@octagonal.nl>

Art-Ev changed the title ~~INRETS and bpr2 dv functions~~ INRETS and bpr2 VD functions Oct 20, 2021

Art-Ev added 2 commits October 20, 2021 15:51

French INRETS VD function

68745ef

First implementation of the French INRETS Volume Delay function. Beta is not used but kept for better consistancy across VD functions.

pedrocamargo requested review from janzill and pedrocamargo October 21, 2021 10:15

pedrocamargo reviewed Oct 21, 2021

View reviewed changes

pedrocamargo requested changes Oct 21, 2021

View reviewed changes

Style issues 1

7df6a8c

Correction of style issues base on flake8 for .py files (E501, line too long , is ignore). Do we need to correct all E501 ? Anything to check for pyx and rst files ?

janzill reviewed Oct 21, 2021

View reviewed changes

Indentation correction and inrets test

cc1e90f

Art-Ev changed the base branch from master to develop October 25, 2021 07:30

Fix break line error

8eb6986

pedrocamargo self-requested a review November 18, 2021 06:09

pedrocamargo approved these changes Nov 18, 2021

View reviewed changes

pedrocamargo merged commit 713f481 into AequilibraE:develop Nov 18, 2021

pedrocamargo mentioned this pull request Mar 30, 2022

Release version 0.7.3 #300

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INRETS and bpr2 VD functions #273

INRETS and bpr2 VD functions #273

Art-Ev commented Oct 20, 2021

Art-Ev commented Oct 20, 2021

pedrocamargo commented Oct 20, 2021

Art-Ev commented Oct 21, 2021

pedrocamargo commented Oct 21, 2021

pedrocamargo Oct 21, 2021

pedrocamargo Oct 21, 2021

Art-Ev Oct 21, 2021

pedrocamargo commented Oct 21, 2021

pedrocamargo Oct 21, 2021

pedrocamargo left a comment

janzill left a comment

pedrocamargo commented Oct 21, 2021

pedrocamargo commented Oct 21, 2021

Art-Ev commented Oct 21, 2021

Art-Ev commented Oct 21, 2021

janzill commented Oct 21, 2021

pedrocamargo commented Nov 8, 2021

Art-Ev commented Nov 8, 2021 •

edited

janzill commented Nov 10, 2021

pedrocamargo commented Nov 15, 2021

INRETS and bpr2 VD functions #273

INRETS and bpr2 VD functions #273

Conversation

Art-Ev commented Oct 20, 2021

Art-Ev commented Oct 20, 2021

pedrocamargo commented Oct 20, 2021

Art-Ev commented Oct 21, 2021

pedrocamargo commented Oct 21, 2021

pedrocamargo Oct 21, 2021

Choose a reason for hiding this comment

pedrocamargo Oct 21, 2021

Choose a reason for hiding this comment

Art-Ev Oct 21, 2021

Choose a reason for hiding this comment

pedrocamargo commented Oct 21, 2021

pedrocamargo Oct 21, 2021

Choose a reason for hiding this comment

pedrocamargo left a comment

Choose a reason for hiding this comment

janzill left a comment

Choose a reason for hiding this comment

pedrocamargo commented Oct 21, 2021

pedrocamargo commented Oct 21, 2021

Art-Ev commented Oct 21, 2021

Art-Ev commented Oct 21, 2021

janzill commented Oct 21, 2021

pedrocamargo commented Nov 8, 2021

Art-Ev commented Nov 8, 2021 • edited

janzill commented Nov 10, 2021

pedrocamargo commented Nov 15, 2021

Art-Ev commented Nov 8, 2021 •

edited