Skip to content

Conversation

@sdmccabe
Copy link
Collaborator

See #239.

Copy link
Collaborator

@leotrs leotrs left a comment

Choose a reason for hiding this comment

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

at this point I'd recommend aliasing i.e. doing something like create_graph = nx.fast_gnp_random_graph(...) at the top of everything so we're sure we caught all of the instances of 0.1 and changed them to 0.3. I'm approving anyway...

@sdmccabe
Copy link
Collaborator Author

The tests are failing on denser graphs.

=================================== FAILURES ===================================
_____________________________ test_directed_input ______________________________
    def test_directed_input():
        with warnings.catch_warnings():
            warnings.filterwarnings(
                "ignore", message="Coercing directed graph to undirected."
            )
            G = nx.fast_gnp_random_graph(100, 0.3, directed=True)
    
            for label, obj in distance.__dict__.items():
                if isinstance(obj, type) and BaseDistance in obj.__bases__:
                    dist = obj().dist(G, G)
>                   assert np.isclose(dist, 0.0)
E                   assert False
E                    +  where False = <function isclose at 0x7f8a4b1d2268>(nan, 0.0)
E                    +    where <function isclose at 0x7f8a4b1d2268> = np.isclose
test_distance.py:88: AssertionError

@leotrs
Copy link
Collaborator

leotrs commented Jun 21, 2019

Do we know which is the offending distance?

@sdmccabe
Copy link
Collaborator Author

QuantumJSD is the offending distance.

@sdmccabe
Copy link
Collaborator Author

sdmccabe commented Jun 22, 2019

My knee-jerk reaction is that QuantumJSD should coerce to undirected, since the definitions in the paper I think tend to mention undirected graphs, and it's using the Laplacian.

This doesn't explain why upping the density caused a problem, of course.

@leotrs
Copy link
Collaborator

leotrs commented Jun 22, 2019

Anything that uses the Laplacian should coerce undirected tho.

@sdmccabe sdmccabe merged commit 57f700d into netsiphd:master Jun 24, 2019
@sdmccabe sdmccabe deleted the test-tweaks branch June 24, 2019 19:27
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.

2 participants