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

Select link cwf bfw bug #500

Merged
merged 7 commits into from
Feb 2, 2024

Conversation

Jake-Moss
Copy link
Contributor

Addressing the bug report raised in #493

Current thoughts are that the blending of the previous and current iteration is failing. The fix in 4894f61 doesn't seem to be enough, though it does extend the number of iterations where the results agree from 1 to 4.

@Jake-Moss Jake-Moss self-assigned this Jan 31, 2024
@Jake-Moss Jake-Moss marked this pull request as draft January 31, 2024 04:22
@Jake-Moss
Copy link
Contributor Author

Overall the root of the issue was that the select link wasn't taking into account the previous step directions in it's
calculations. In principle, the patch that modified the select to do just that should have worked, however, the matrix
that stored the previous step directions was allocated on the wrong object and, while the object lived the entire
duration of the assignment, the matrix was being zeroed at each iteration. I'd overlooked this as it did improve the
stability up to 4 iterations, but Jan pointed out to me that the beta's being used were 1.0 (and 0.0 respectively) up
until the 5th iteration where the step direction truly started to matter.

Another two bugs I found are patched.

  1. The aon object was being leaked outside of the traffic class for loop, this meant that the last traffic class was
    being used for all select links analysis'. The aon objects are now stored and reused, this may increase memory usage
    during the assignment as GC can no longer clean the memory allocated by these objects up at the end of every AON
    assignment, instead it will at the end of all assignments.

  2. As noted in the comment pre_previous_step_direction was being used in place of previous_step_direction.

Overall the root of the issue was that the select link wasn't taking into account the previous step directions in it's
calculations. In principle, the patch that modified the select to do just that should have worked, however, the matrix
that stored the previous step directions was allocated on the wrong object and, while the object lived the entire
duration of the assignment, the matrix was being zeroed at each iteration. I'd overlooked this as it did improve the
stability up to 4 iterations, but Jan pointed out to me that the beta's being used were 1.0 (and 0.0 respectively) up
until the 5th iteration where the step direction truly started to matter.

Another two bugs I found are patched in this commit.

1. The aon object was being leaked outside of the traffic class for loop, this meant that the last traffic class was
being used for all select links analysis'. The aon objects are now stored and reused, this may increase memory usage
during the assignment as GC can no longer clean the memory allocated by these objects up at the end of every AON
assignment, instead it will at the end of all assignments.

2. As noted in the comment `pre_previous_step_direction` was being used in place of `previous_step_direction`.
@Jake-Moss Jake-Moss force-pushed the select_link_cwf_bfw_bug branch from 5623288 to e1376b8 Compare February 1, 2024 05:01
@Jake-Moss Jake-Moss marked this pull request as ready for review February 1, 2024 05:59
Copy link
Contributor

@janzill janzill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, nice catch on the cfw pre_previous bug. To clarify, this should not influence results much because it only matters when running bfw and then only on the first bfw step-direction calculation after a cfw step (the latter has to happen when building up the previous and pre-previous step direction, so only once intitially or whenever the step-direction resets).

@pedrocamargo pedrocamargo merged commit f89c19f into AequilibraE:develop Feb 2, 2024
25 checks passed
@pedrocamargo pedrocamargo deleted the select_link_cwf_bfw_bug branch February 2, 2024 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants