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

Cache values in the RC system for speed #1579

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Cache values in the RC system for speed #1579

wants to merge 1 commit into from

Conversation

hunse
Copy link
Collaborator

@hunse hunse commented Nov 4, 2019

Motivation and context:

Previously, repeated rc.get calls could take significant time. I noticed this when profiling. However, I'm not sure where the particular calls are in our code (I know they're in the simulator step function somewhere, either here or in nengo-loihi).

My solution was to cache things in our _RC class when they're first used. I've also configured the cache to get cleared (for the specific value) when rc.set is called, or for all values if we clear/reload the RC file. This should ensure that users can still use rc.set or load their own RC file before making networks/simulators/etc., and the new settings will be used.

We should probably go through and make sure when we make operators for a simulator, they're not calling into rc repeatedly (they should load the rc value once when they make the operator, and then re-use the value). From what I can tell, the place where it is actually happening is in SupportDefaultsMixin.__setattr__, where we check if we're using simplified exceptions. It might be a little tricky to cache things there, because we don't want to cache to early (e.g. if some config setting is set when Nengo is imported) and have a later user change of the value not have an effect. This is why I've taken the current approach for now.

How has this been tested?

I've ran the test suite. We'll need some unit testing to ensure full coverage.

How long should this take to review?

  • Average (neither quick nor lengthy)

Types of changes:

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • I have updated the documentation accordingly.
  • I have included a changelog entry.
  • I have added tests to cover my changes.
  • I have run the test suite locally and all tests passed.

Still to do:

  • Add unit tests; ensure full coverage.

@hunse hunse mentioned this pull request Nov 8, 2019
5 tasks
Previously, repeated `rc.get` calls could take significant time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant