search

SpiNNakerManchester / spalloc_server

A SpiNNaker machine allocation and partitioning application.
0 stars 2 forks source link

Switch controller to spinnman #7

Closed dkfellows closed 7 years ago

dkfellows commented 7 years ago

Converts spalloc to use an enum for link directions that is defined inside spalloc instead of using a version from rig.

Currently some problems though, as there's additional coordinates in the SpiNNMan code that Rig lacked. What should the cabinet and frame be?

mossblaser commented 7 years ago

Hey there! Really glad to see spalloc has a new maintainer! Do feel free to ping me any questions you might have, the developer docs are, sadly, not battle tested (and I'm fairly inexperienced in these matters and would be grateful to hear what sort of things have been omitted or were unclear).

Just a quick note, it appears the tests have recently started failing in master. The test suite should be fairly comprehensive and for a program with such a large state space (and a fairly mission critical role) I'd strongly encourage you to keep it up to date. If it helps, spalloc_server uses a similar setup/ethos to rig for tests and building documentation which you can read about here: https://github.com/project-rig/rig/blob/master/DEVELOP.md If you're having trouble getting tests working properly then please don't suffer in silence and let me know :)

Once again, good luck with the new role! Glad to see the SpiNNaker team growing again too! I rather miss the place!

rowleya commented 7 years ago

Just having a look at the Travis tests - the issue seems to be with dependencies of dependencies; SpiNNMan requires SpiNNStorageHandlers and this doesn't seem to get detected correctly (SpiNNMan seems to have been added correctly though). Is this a known issue on Travis?

dkfellows commented 7 years ago

Travis is a bit finicky because the environment it runs in is a little odd, but if that is the main problem then it's easy to fix (I used the same techniques with SpiNNMan). The upside is that once I've fixed that, it will be much easier to on-board new developers as we'll have the concrete dependency requirements described.

rowleya commented 7 years ago

Fixing is a good idea - but I still don't see why it doesn't work! SpiNNStorageHandlers is already a dependency of SpiNNMan and it installs SpiNNMan. The only reason I could see for it not working is if the >=3.0.0 and <=4.0.0 was not accurate. I am fairly sure this is correct, but if this is the issue, the dependency should definitely be updated.

dkfellows commented 7 years ago

@mossblaser I know about the failures on master; I think they're because I'm in the process of switching the code over from using inotify to signals (specifically SIGHUP) as that's more in line with how Unix daemons operate.

I guess I should have put it on a branch though. Oh well.

mossblaser commented 7 years ago

Hey there. I'm not sure what oddities of Travis you're finding but it seems that the issue here (looking at the travis log) is that the wrong version of SpiNNMan is being installed:

https://travis-ci.org/SpiNNakerManchester/spalloc_server/jobs/195867335#L185

Assuming what's in SpiNNMan's setup.py is in line with the latest release, it should be installing 3.0.0 but is instead installing 2016.1. Putting the required version range in the requirements should help.

Once fixed, the other issue which looks like it will cause the tests to fail is that SpiNNMan still appears to be incompatible with Python 3.

Incidentally, what version of Python/pip are you using? Both Python 2.7 and 3.5's version of pip on my machine seem to install the wrong version.

rowleya commented 7 years ago

Ahh yes, I see the issue with the version now - it should be SpiNNMan >= 3.0.0 and < 4.0.0. With that, it should then work.

I am not too concerned with Python version 3 support. I would prefer to disable this in spalloc than support it in SpiNNMan at present.

mossblaser commented 7 years ago

I am not too concerned with Python version 3 support. I would prefer to disable this in spalloc than support it in SpiNNMan at present.

:( Don't forget to update setup.py to reflect this.

By the way, are there any new/exciting features are on the horizon for spalloc by the way that have kicked off this new activity?

dkfellows commented 7 years ago

Part of the issue is that master has some issues (which I'm fixing) and this branch has others.

rowleya commented 7 years ago

My change to include the version number fixes the import errors but as stated, there are other errors. As this is being worked on, I will wait until these are fixed before looking at this further.