networkx / networkx-metis

NetworkX Addon to allow graph partitioning with METIS
Other
79 stars 21 forks source link

Add appveyor.yml and necessary files to build tests on AppVeyor #39

Closed OrkoHunter closed 9 years ago

OrkoHunter commented 9 years ago

This is more or less how sckit-learn does it. I've removed unnecessary parts and we can do more but first we need to setup the hook on https://ci.appveyor.com to start the first build. @chebee7i Can you setup the hook for networkx-metis?

MridulS commented 9 years ago

@OrkoHunter did you build this on appveyor. This doesn't seem to work for me.

OrkoHunter commented 9 years ago

Yes, there's a C library build problem. msvc seems to be throwing lots of build failures which gcc as a compiler have been accepting. Here's the build log of my fork.

ysitu commented 9 years ago

The problem is in https://github.com/networkx/networkx-metis/blob/master/src/GKlib/gk_types.h#L22. ssize_t is not defined in MSVC. You need to add something like

#ifdef _MSC_VER
#include <basetsd.h>

typedef SSIZE_t ssize_t;
#endif
chebee7i commented 9 years ago

networkx-metis should be ready for commits now.

OrkoHunter commented 9 years ago

Container-based Travis environment has become really fast.

ysitu commented 9 years ago

@OrkoHunter Will you help port the Travis CI changes to networkx?

OrkoHunter commented 9 years ago

Yes, I'm sending a PR shortly. I was testing the changes on my fork.

MridulS commented 9 years ago

@OrkoHunter Are you also working on making AppVeyor testing for networkx?

OrkoHunter commented 9 years ago

@OrkoHunter Are you also working on making AppVeyor testing for networkx?

As soon as it gets working with networkx-metis, it can be ported to networkx.

OrkoHunter commented 9 years ago

The latest error on the build is

src/GKlib\gk_types.h(24) : error C2371: 'ssize_t' : redefinition; different basic types 
        c:\projects\networkx-metis\src\gklib\gk_arch.h(53) : see declaration of 'ssize_t' 

https://ci.appveyor.com/project/OrkoHunter/networkx-metis/build/1.0.27/job/h9lgxxsgahc27gwf#L399

ysitu commented 9 years ago

Like I said, after changing WIN32 to _WIN32, you should be able to revert the change to gk_types.h.

OrkoHunter commented 9 years ago

Ah! Sorry.

OrkoHunter commented 9 years ago

@ysitu Is it possible to cancel all the builds except for the latest one? https://ci.appveyor.com/project/chebee7i/networkx-metis-1lf0f/history I think @chebee7i could do it only because it's under project/chebee7i

ysitu commented 9 years ago

Apparently @chebee7i owns the CI as you said, so I can do nothing.

OrkoHunter commented 9 years ago

@chebee7i if you will cancel all the builds except for the last one, probably 7-8 hours of time can be saved :)

chebee7i commented 9 years ago

Hmm...unfortunately, it doesn't appear that appveyor allows me to give others access (as readthedocs does). I will go ahead and cancel, but we should try to figure out a better solution for "organizations".

chebee7i commented 9 years ago

It might be worth emailing them about it. I will be travelling tomorrow for most of the day, so I expect to be unavailable.

chebee7i commented 9 years ago

I cancelled all builds, please initiate a new build.

OrkoHunter commented 9 years ago

Thank you. Yeah, I really like how readthedocs gives the moderators option. I'll try bringing it in there concern.

ysitu commented 9 years ago

This is looking good. Please update NOTICE to include the additional changes you have done to METIS and also credit the authors of install.ps1 and run_with_env.cmd there.

OrkoHunter commented 9 years ago

The test for part_graph function failed on AppVeyor. It's giving out a different answer than expected. Also, it's strange that test passes on Travis.

ysitu commented 9 years ago

Well, METIS does not guarantee that it will always generate the optimal solution since it is an approximation algorithm to NP-hard problems. We need to allow some variation in the test output.

OrkoHunter commented 9 years ago

Okay then. I'll update the NOTICE.

OrkoHunter commented 9 years ago

Any more changes in the NOTICE or correction?

ysitu commented 9 years ago

That looks good to me. Let's fix the test in a separate PR.