Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timestep changes #272

Closed
wants to merge 64 commits into from
Closed

Timestep changes #272

wants to merge 64 commits into from

Conversation

Christian-B
Copy link
Member

@Christian-B Christian-B commented Nov 28, 2019

This is an alternative solution for different timesteps per vertex.

Before this PR is merged we need to decide.

  1. Do we at the python level wish to support multiple timesteps.
  2. Do we want to support Application Vertexes with more than one timestep.

The current state of the PR is to support both 1 and 2 as yes.
I @Christian-B prefer to force an ApplicationVertex to have just a single timestep which would require a cleanup here before merging.

Must be done at the same time as:
SpiNNakerManchester/SpiNNFrontEndCommon#528
SpiNNakerManchester/SpiNNakerGraphFrontEnd#134
SpiNNakerManchester/lab_answers#4
SpiNNakerManchester/sPyNNaker#749
SpiNNakerManchester/sPyNNaker8#317
SpiNNakerManchester/sPyNNakerNewModelTemplate#48

This effects a HUGH amount of stuff so I wonder if having different "timestep"s for different vertexes is a good idea.
An Alternative is to have only a single Python timestep which is used EVERYWHERE with the C code of the vertexes running at a different speed doing any conversions.
With possibly minimum python changes to make sure the runtime is rounded up to a lowestCommonMultiple of the C runtimesteps.

@coveralls
Copy link

coveralls commented Nov 28, 2019

Coverage Status

Coverage increased (+0.3%) to 68.319% when pulling d9333d3 on timestep into 67eae83 on master.

@Christian-B Christian-B changed the title Replace the timestep params with simtime Timestep changes Feb 12, 2020
@Christian-B Christian-B added the incomplete_left_for_further_release Not ready for merging label Feb 18, 2020
@Christian-B Christian-B modified the milestones: 6.0.0, 7.0.0 Jun 4, 2020
@Christian-B Christian-B added historic Of historic interest only and removed incomplete_left_for_further_release Not ready for merging labels Mar 31, 2021
@Christian-B
Copy link
Member Author

Too many changes too complex to be worth it,
Leaving branch for histocial reasons.

@rowleya rowleya deleted the timestep branch October 5, 2023 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement historic Of historic interest only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants