Only use idle cores

[mossblaser]

This PR began as an answer to #202 to attempt to automatically avoid using non-idle cores and has resulted in a few wider-scoped changes to make this process cleaner.

• get_machine has been deprecated since having the MachineController directly handling P&R artefacts seems like a bad idea in general.
• get_chip_info uses a new SCP command to quickly return basic information about the health and state of a chip.
• A new method, get_system_info, and associated SystemInfo container has been added which uses get_chip_info to quickly gather the state of every chip, link and core in a SpiNNaker machine. The SystemInfo contains a superset of the information Machine did and does so in a far more explicit way. This call is also a little quicker than get_machine was.
• A utility function build_machine has been added which builds a Machine object from a SystemInfo when you are doing place-and-route work.
• A utility function build_core_constraints has been added which builds P&R constraints which reserve all non-idle cores in a machine (including monitors) using information in a SystemInfo object.
• The old place-and-route wrapper has been deprecated in favour of a new wrapper which doesn't place things on non-idle cores. It also takes a SystemInfo object in place of a Machine.
• Machine and its associated sentinels now live in rig.place_and_route.machine and are aliased in rig.place_and_route.
• Links now lives in rig.links.
• For backward compatibility, wrappers exist for the old functions, methods and API structures, the use of which will trigger a deprecation warning.

Once this PR is merged and released, I'll submit patches for other projects.

• Nengo SpiNNaker
• pinus_rigida
• network_tester
• rig_examples

[mossblaser]

OK Since this PR is fully backward compatible with older versions of Rig I'd suggest merging it in and I'll work on PRs for other projects in due-course since this PR won't break anything.

[mundya]

I still need to read the tests... sorry

[mossblaser]

Thanks very much for all the reviewing, take your time! I'll get on the corrections...

I've been thinking more about all this and I've come to the conclusion some things about the API coming out of this PR leave me feeling quite uncomfortable:

With the API exposed as it is here you cannot trivially answer the question "how many idle cores are there in total" without doing your own get_chip_info which is wasteful and doesn't feel right. As a result I think the _get_all_chips_info method should be made public. It should probably get a better name too...

I do not like the fact that get_resource_constraints, a MachineController method, is returning place-and-route constraints, a construct designed for a very specific application and with little general use outside that context. Additionally, if you already have a ChipInfo for every chip there is no reason for get_resource_constraints to communicate with the machine and thus no reason for it to be part of MachineController. Instead I believe this should live with the other place-and-route utility functions.

Perhaps more controversially get_machine is essentially exactly the same in that once given a ChipInfo it does not actually need to communicate with the machine at all. Though the Machine object is carefully designed to actually be quite useful outside of place-and-route, it still has plenty of quirks related to the place-and-route infrastructure (e.g. the concept of 'resources' and the Cores, SDRAM and SRAM sentinels). All this points towards get_machine being pulled out and put into a place-and-route utility function and even the Machine object being pulled into the P&R namespace. Obviously a deprecation-flagged wrapper in MachineController must remain along with aliases for the old type name for backward compatibility.

Sadly going from the Machine object to a bare dictionary of ChipInfos feels like a regression in high-level functionality in many ways. To relieve some of this I'd propose making get_all_chips_info return a dict-like object with extra functionality beyond the usual lookup from chip to ChipInfo. The API should hopefully recreate the useful functions of the Machine object, namely easy iteration over (live) chip and link coordinates, easy testing for chip and link liveness, directly getting the width/height of the network. Further functionality which wouldn't have been appropriate for Machine objects would be things like iterating over core coordinates, or even cores in a particular state.

So, what do you think?

• Is the above sensible?
• If so, what should the chip-information-holding object be called? It needs to be short/snappy and not be confused for Machine...

[mossblaser]

OK; following the above, the following things have happened:

• A new get_system_info method replaces get_machine, get_resource_constraints and all that nonsense.
• A new SystemInfo type has been introduced which is a dict from (x, y) to ChipInfo along with some utility methods for iterating over things and checking membership. It should replace Machine objects in settings not related to P&R.
• get_machine is now deprecated (but still present for backward compatibility).
• A pair of P&R utility methods build_machine and build_core_constraints have been provided which use a SystemInfo object to build those data structures.
• The new place_and_route_wrapper now takes a SystemInfo as an argument.

I think some reorganisation still needs to happen with the Rig module hierarchy:

• [x] The Machine type and associated sentinels should be moved into the place_and_route namespace.
• [x] The Links type probably needs a better home than the only member of rig/machine.py...

[mossblaser]

OK, I think this PR is now finally complete and ready for final review and merging! The comment at the top of the PR should hopefully enumerate the changes made. Sadly due to the light internal reorganisation this PR has touched more code than it would otherwise need to.

[mundya]

My read through the tests was cursory, but this looks mostly good to me. I am concerned that on big machines it'll generate huge number of resource constraints, but I may just be misunderstanding that bit of code.

[mossblaser]

I am concerned that on big machines it'll generate huge number of resource constraints, but I may just be misunderstanding that bit of code.

See previous comment.

TODOs for me before I merge this then are:

• [x] Fix misc things pointed out above
• [x] Fix remaining references to "rig.machine"
• [x] Enable generation of global constraints when possible

[mundya]

Enable generation of global constraints when possible

Would an option be allowing constraints to be associated with multiple locations? - That way you could just modify one set of constraints and then mark the constraint as global (remove the location) if the difference between the set of locations the constraint is applied to and the overall set of locations is just the set of dead chips.

new_location = None if (old_locations - all_locations == dead_locations) else old_locations

[mossblaser]

Would an option be allowing constraints to be associated with multiple locations?

I don't think that would meaningfully save you compute or storage costs but would make life more complex. I'll just stick to producing a minimal set of constraints as they stand now.

[mossblaser]

OK; this awaits your final review. Feel free to merge if you're happy!

[mundya]

LGTM, I'll await the tests coming back

[mundya]

Are the changes to rig_par_diagram necessitated by this trivial to make?

[mossblaser]

Hopefully none what so ever. Has it started failing? On 3 Jan 2016 09:13, "Andrew Mundy" notifications@github.com wrote:

Are the changes to rig_par_diagram necessitated by this trivial to make?

— Reply to this email directly or view it on GitHub https://github.com/project-rig/rig/pull/206#issuecomment-168480463.

[mundya]

No, I'm stupid, I'd forgotten that Machine still existed. Sorry!