Skip to content

Test binaries#1637

Closed
Christian-B wants to merge 12 commits intomasterfrom
test_binaries
Closed

Test binaries#1637
Christian-B wants to merge 12 commits intomasterfrom
test_binaries

Conversation

@Christian-B
Copy link
Member

@Christian-B Christian-B commented Feb 5, 2026

requires: SpiNNakerManchester/TestBase#72

Increases number of binaries tested.

  • cfg "n_synapse_cores" mainly to split PyNNExamples without changing the script

Several test tweaked to test both the original binary and the extra ones

test_binary is the fall back when no tweak-able test found.

tested by https://github.com/SpiNNakerManchester/IntegrationTests/actions/runs/21713093809

@Christian-B Christian-B requested a review from rowleya February 5, 2026 12:59
@Christian-B
Copy link
Member Author

Depends on #1641

Copy link
Member

@rowleya rowleya left a comment

Choose a reason for hiding this comment

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

Couple of comments

translator_2 = Translator(devices_2)
model_2 = p.external_devices.ExternalDeviceLifControl(
devices_2, create_edges, translator_2)
model_2.set_model_n_synapse_cores(1)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
model_2.set_model_n_synapse_cores(1)
p.external_devices.ExternalDeviceLifControl.set_model_n_synapse_cores(1)

Might be better to advise against calling a class method on an instance?

Copy link
Member Author

Choose a reason for hiding this comment

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

agree will be changed after #1642 and #1641

Comment on lines +127 to +145
self._add_test_cfg_settings()

def _add_test_cfg_settings(self) -> None:
"""
Ideally this function will find nothing to do.

This is designed for switching scripts like PyNNExamples
to split mode without the need to change the scripts.

Better to call set_model_n_synapse_cores in the script
"""
n_synapse_cores = get_config_str_list("Simulation", "n_synapse_cores")
for n_synapse_core in n_synapse_cores:
path, name, n = n_synapse_core.split(":")
model_class = getattr(importlib.import_module(path), name)
model = model_class()
model.set_model_n_synapse_cores(int(n))
logger.warning(f"model:{name} n_synapse_core set to {n} "
f"based on cfg")
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this can't just be done in the test script itself?

Comment on lines +66 to +71
n_synapse_cores = None
@n_synapse_cores = Testing Option!
A better way is to call set_model_n_synapse_cores on the model.
Mapping of a AbstractPyNNNeuronModel to n_synapse_cores to be set
In the format model:n_synapse_cores,model:n_synapse_cores

Copy link
Member

Choose a reason for hiding this comment

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

See other comments

@Christian-B
Copy link
Member Author

replaced by #1643
as the cfg stuff not needed due to SpiNNakerManchester/PyNNExamples#142

@Christian-B Christian-B deleted the test_binaries branch February 10, 2026 12:30
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