Development of 0.7.3 version

[pedrocamargo]

Current issues

[pedrocamargo]

@Art-Ev , I merged your PR without running all tests because Linting was failing, but I see now that one of the tests for the new VDFs is still failing. Can you take a look at it and put a new PR?

[pedrocamargo]

@janzill , CFW is just not working (doesn't converge at all). Can you take a look at it?

[Art-Ev]

@Art-Ev , I merged your PR without running all tests because Linting was failing, but I see now that one of the tests for the new VDFs is still failing. Can you take a look at it and put a new PR?

• Implementation was initially based on the changes done for Implements Spiess' conical VDF #210. Never worked with cython before, is there something else to do ? How can I generate the same binaries as the ones necessary for the old QGIS plugin ? (using Setup_Assignment.py with OSGeo4W python environnement result in no Python.h file found)

• Trying to normalize indentation through all VDF files I've touch all of them, does replacing bpr.pyx with the old one allow CFW to converge ?

[janzill]

@janzill , CFW is just not working (doesn't converge at all). Can you take a look at it?

Yeah, can re-produce locally, I had a quick look at the PR diff and it doesn't look like there is a change compared to master (where things are working) that should break it. I'll have a more detailed look later, I'll see if indentation matters @Art-Ev.

[janzill]

so just reverting bpr whitespace fixes the convergence issue, although now the comparison between bfw and cfw rgap is failing. It doesn't really make sense to compare these if we don't hit the maximum number of iterations I believe, we should be comparing the number of iterations it took to converge, and if they are equal to the maximum number of iterations, we can compare the rgap. I just don't know why this hasn't shown up before.

[janzill]

Just compared the test project cfw and bfw convergence behaviour with master, those results (in terms of step size and rgap periteration) are slightly different. I have no idea why, it's small enough to be numerical precision potentially, but I cannot see where this could happen based on the master/develop diff. I'll look into results (i.e. link volumes) when I get a bit of time.

[pedrocamargo]

• Implementation was initially based on the changes done for Implements Spiess' conical VDF #210. Never worked with cython before, is there something else to do ? How can I generate the same binaries as the ones necessary for the old QGIS plugin ? (using Setup_Assignment.py with OSGeo4W python environnement result in no Python.h file found)

@Art-Ev , in order for this to work with the old version of the plugin you would have to compile for your target Python version, but many other changes would also be needed, as now there are other imports that are not available in QGIS (like Feather), which I am slowly going through...

[Art-Ev]

so just reverting bpr whitespace fixes the convergence issue, although now the comparison between bfw and cfw rgap is failing. It doesn't really make sense to compare these if we don't hit the maximum number of iterations I believe, we should be comparing the number of iterations it took to converge, and if they are equal to the maximum number of iterations, we can compare the rgap. I just don't know why this hasn't shown up before.

About whitespace : I will make a PR for the other VDFs. Will take a look later at the exact problem (maybe cython requires to work differently from python about line breaks?)

[Art-Ev]

I cannot achieve checks but for convergence issue, there are some VDFs versions to test if the indentation problem is only related to congested_time/deltaresult computation or if it's more general:
VDFs_test1.zip
Can somebody make a quick test from that ?

[pedrocamargo]

@Art-Ev , I ran the tests locally, and there is an issue with the derivative of the inrets around V/C equal to 1.0.

Basically, the derivative goes from 5.5 to 3.0000003 when the V/C goes from 1.0 to something larger than 1.0. The assumption of the test is that the derivative needs to be monotonically increasing, which I guess may not be the case around 1.0 indeed.

[Art-Ev]

@pedrocamargo it seems that INRETS vdf cannot match this test by definition as it have a different definition before/after V/C=1. (or maybe my math is just wrong). In fact, there is also the same case for BPR2 vdf (there is just no test yet for it).

Does this sudden change in the derivative can be a problem during affectation ? (for convergence)
If not, we can just delete derivativ test or built a two part test (before and after V/C=1)

[pedrocamargo]

@pedrocamargo it seems that INRETS vdf cannot match this test by definition as it have a different definition before/after V/C=1. (or maybe my math is just wrong). In fact, there is also the same case for BPR2 vdf (there is just no test yet for it).

Does this sudden change in the derivative can be a problem during affectation ? (for convergence) If not, we can just delete derivativ test or built a two part test (before and after V/C=1)

I think it is solved, @Art-Ev #287

[pedrocamargo]

It is solved, @Art-Ev . The new VDF functions you brought in will be in the next version, which should be released in the coming weeks.

[pedrocamargo]

@janzill , @jamiecook Anything we should consider before merging this and making a new release? I guess this is where more thorough testing (maybe full workflows) should come, but that's in the future...

[janzill]

I think we resolved everything, except for running tests on macos, right?