[py3] core_tests.test_threshold_proportional FAIL about bctpy HOT 8 CLOSED

I probably have the most up-to-date py3 branch here https://github.com/clbarnes/bctpy/tree/py3 as it's not a priority for @aestrivex . Although all I've been doing is

#!/bin/bash
set -e
if [ "$(git symbolic-ref --short -q HEAD)" = "master" ]; then
    git checkout py3
    git checkout master -- .
    git commit -a -m "update from master"
    2to3 -W -n bct test setup.py
    git commit -a -m "run 2to3"
    nosetests --ignore-files="very_long_tests|nbs_tests""
    git checkout master
fi

Unfortunately, this means the merges aren't fast-forwardable, which means it's a bit of hassle for aestrivex each time.

In this test, the sum is off by 4 (probably 3 edges). I'll have a look into it now.

from bctpy.

There are a lot of other tests which depend on the threshold_proportional method which pass, which is interesting...

from bctpy.

I will try and look at this in a few days, but I don't have time right now.

Thanks for all your help @clbarnes

from bctpy.

Don't worry, I should have an answer in the next hour or so, I'm just noting stuff down here as I go (sorry if that's peppering you with emails!).

The difference is that the py3 version zeros one more element than py2. That element is [107, 111], and has value 1.85 (which is doubled when the matrix is re-symmetrised). I'm digging into the root cause now; I suspect it's something to do with this line en = int(round((n * n - n) * p / ud)) rounding either up or down after the division.

from bctpy.

Here we go!

py3's round(0.5) rounds towards the even choice (i.e. 0). py2's round(0.5) rounds away from 0 (i.e. 1). The py3 version is in accordance with the IEEE-754 specification default, R, and .NET, and removes sign bias as well as making the mean of a large set of rounded numbers the same as the mean if they were not rounded.

The pythonic fix would be to make the py2 version depend on future, and then do from builtins import round (alternatively, use np.round(), as this has the new behaviour in both versions). This opens the door to writing more cross-compatible code in future, thus cutting down on effort required to maintain the two versions. It also means that using 2to3 to maintain the two versions is still viable. Also, because np.round() is already used in some places in the code, it would improve consistency.

However, this would be a departure from the behaviour of matlab, which rounds in a py2-like fashion.

It is also possible to make the py3 version depend on past, and use the builtins from that package. However, this change which would need to be checked every time the py3 codebase was regenerated with 2to3.

from bctpy.

I think it is better to keep as much compatibility from the matlab version where possible, and in the past when I have come across similar choices I have tried to always get the similar behavior. That is probably why I chose to use python stdlib round and not np.round in the first place.

Making py3 branch not work very easily with 2to3 doesn't sound like a good solution either.

What do you think of simply using a custom (integer only) round that does the right thing?

from bctpy.

That's a good option, although there still might be some minor inconsistencies as numpy.round() is used in a few other places in the reference module.

from bctpy.

Fixed by implementing teachers round

from bctpy.