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

protected Cpu info methods #358

Closed
wants to merge 15 commits into from
Closed

protected Cpu info methods #358

wants to merge 15 commits into from

Conversation

Christian-B
Copy link
Member

@Christian-B Christian-B commented Aug 18, 2023

This PR looks at the Transceiver functions
get_cpu_information_from_cores and get_cpu_infos
to see why these are called.

Turns out the usages where

  1. get_region_base_address
  2. get_cores_in_states
  3. get_cores_in_not_states
  4. Print a String of the cores in/ not in states

So instead the methods that return detail CPUInfo objects have need protected and instead methods 1 to 3 have been added.

Methods 2 and 3 currently still return CPuInfo objects BUT only their iter method of this Object is used
This allows the Spin2 to do it differently if desired

Method 4 can actually be replicated by calling methods 2 or 3 using the x,y,p iter to a fill a set (or list) and then str that

There is a minor disadvantage that is rare cases
ChipProvenanceUpdater if it is going to call an Exception then it needs to make a few extra calls
Stand alone script get_cores_in_run_state also needs a few extra calls

Must be done at the same time as:
SpiNNakerManchester/SpiNNFrontEndCommon#1104
SpiNNakerManchester/sPyNNaker#1379
SpiNNakerManchester/SpiNNakerGraphFrontEnd#257

tested by:SpiNNakerManchester/IntegrationTests#230

@coveralls
Copy link

coveralls commented Aug 18, 2023

Coverage Status

coverage: 50.332% (+0.2%) from 50.172% when pulling a56b228 on cpu_info into 8c40423 on master.

@dkfellows dkfellows added this to the 7.1.0 milestone Sep 27, 2023
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.

I think this isn't quite finished because of the assertion...

Comment on lines 1001 to 1008
a = self.read_user(x, y, p, 0)
core_subsets = CoreSubsets()
core_subsets.add_processor(x, y, p)
process = GetCPUInfoProcess(self._scamp_connection_selector)
cpu_info = process.get_cpu_info(core_subsets)
b = cpu_info.get_cpu_info(x, y, p).user[0]
assert a == b
return a
Copy link
Member

Choose a reason for hiding this comment

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

Seems odd to go to the trouble of doing this twice!

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 debug do it both ways. Removed

@Christian-B Christian-B mentioned this pull request Oct 9, 2023
@Christian-B
Copy link
Member Author

Replaced with #366

@Christian-B Christian-B deleted the cpu_info branch October 10, 2023 06:36
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.

4 participants