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

easier-to-understand way to go back to default refclk settings #100

Merged
merged 7 commits into from Dec 3, 2022

Conversation

kevinkiener
Copy link
Contributor

When reloading the QickSoC with an external_clk=False the setting did not change because the if condition was not fulfilled. Now it checks whether the clocks have previously been set to an external_clk or not.

@meeg
Copy link
Collaborator

meeg commented Dec 1, 2022

As things stand, if there's a possibility that the clock settings have been changed you need to set force_init_clks = True to reload the default settings. Certainly this is not ideal.

This fix doesn't work in all cases - it works if the libraries remain loaded and you're just reloading the QickSoc, but it doesn't work if you're reloading the library as well (restarting your notebook/script, or working in a different notebook). In that case the xrfclk library will have been reloaded, and the check will always set clock_initialized = False.

Given that there is no obvious way to make the software always do the right thing, I would prefer to leave things as they are (you don't need force_init_clks = True to change the clock settings, but you always need force_init_clks = True to restore normal settings) or go back to how things were when you submitted #99 (you always need force_init_clks = True to change or restore). It seems more confusing to add this PR (you sometimes need force_init_clks = True to restore normal settings).

Thoughts?

@meeg
Copy link
Collaborator

meeg commented Dec 1, 2022

Another possible tweak is to make it so specifying any value (either True or False) for external_clk triggers a forced reload. Would that work for you?

@kevinkiener
Copy link
Contributor Author

Yes, that's a good idea, so I can change it to have a default value of None and then if the user sets it to true or false we will reset the clocks/ force reload

@meeg meeg merged commit 3a152db into openquantumhardware:main Dec 3, 2022
@meeg meeg changed the title bug fix from external_clk = True to external_clk = False easier-to-understand way to go back to default refclk settings Dec 3, 2022
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.

None yet

2 participants