Fix vdf not calculated last iteration of assignment

[barisdemirdelen]

In linear_approximation.py, vdf calculation happens after convergence is checked. If an assignment is converged the function breaks out of the loop, that means no vdf calculation can be made for the last iteration of the assignment.

This pull request moves applying the vdf before convergence checking and breaking, so that the last iterations vdf can also be calculated before assignment finishes.

[pedrocamargo]

Excellent catch. @janzill , can you take a second look before we merge it?

[pedrocamargo]

Tests that fail are due to the lack of GitHub secrets in the contributing repo, so all good

[janzill]

Thanks @barisdemirdelen , good catch. I think we still need to change this slightly and move the calculation of the vdf after the convergence metric calculation to not use the updated costs. We will also want the convergence report to be updated, so my suggestion is to move the convergence check (l428 in your code) to the end of the the iteration loop, and also invert the application of the vdf and the calculation of convergence statistics. Does that make sense?

[barisdemirdelen]

Thanks @barisdemirdelen , good catch. I think we still need to change this slightly and move the calculation of the vdf after the convergence metric calculation to not use the updated costs. We will also want the convergence report to be updated, so my suggestion is to move the convergence check (l428 in your code) to the end of the the iteration loop, and also invert the application of the vdf and the calculation of convergence statistics. Does that make sense?

For the first suggestion, I think it makes sense to put the l428 if to the end of the loop.
For the second suggestion, do you mean in check_convergence() we actually want the old self.congested_time and not the updated one by the vdf?

[janzill]

For the second suggestion, do you mean in check_convergence() we actually want the old self.congested_time and not the updated one by the vdf?

Yes, this is on purpose, I think we want the costs that were used to calculate the current all-or-nothing