SpiNNakerManchester / SpiNNFrontEndCommon

Common support code for user-facing front end systems.
Apache License 2.0
12 stars 11 forks source link

PairOnChipRouterCompression creating merges that overlap #1180

Open Christian-B opened 7 months ago

Christian-B commented 7 months ago

Detected when running TSPonSpiNNaker/examples/tsp.ipynb with cfg router_table_compress_as_far_as_possible = True

Method find_merge https://github.com/SpiNNakerManchester/SpiNNFrontEndCommon/blob/407ee42795dc1033ba7fde4cf59423c23d3b67b4/c_common/models/compressors/src/compressor_includes/pair_minimize.h#L109 Should before doing a merge check that it does not overlap later routes. This was failing because route_cache[cache] held the wrong data.

The optimised way https://github.com/SpiNNakerManchester/SpiNNFrontEndCommon/blob/62d280e1afb285a1a8e9e03627bca818e1413ab0/c_common/models/compressors/src/compressor_includes/pair_minimize.h#L83

Does work.

Christian-B commented 3 months ago

Farther checking reveals that the Routing tabke entires returned by pairminimize.h transfer_next method are wrong

This was fancy stuff for when we had very large routing tables that where stored in ssram and could only be transfered in blocks.

There are tow options.

1.Strip out the broken code as there is now only one routing table implementation rt_simple.h find_merge_optimised transfer_next routing_table_get_entries routing_table_wait_for_last_transfer dma_done()/ dma_done(void) (and more)?

  1. just comment the ocde as broken and leave it

  2. Try to fix the code

Options 2 and 3 only make sense if we plan on reintroducing routing tables that need to be transferred in blocks.