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

Added more clarity to units #14

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

clarkmiyamoto
Copy link

Thought elaborating on the units might help a new front-end user a little more, especially since the superconducting literature is split between linear and angular units.

  • Stated this package assumes planks constant $h = 1$
  • Everything is in linear freq
  • Gave a few examples

Let me know if I got anything wrong and I'll update it! Thanks again for maintaining such a useful package!

@jkochNU
Copy link
Member

jkochNU commented Jul 19, 2023

Hi @clarkmiyamoto - awesome to get your contribution here, thank you! I'd like to ask @petergthatsme for input before merging, as he is the origin of all units choices and functionality in scqubits.
I'd like to inject my own opinion on two points:

  1. The statement that literature usually specifies EC/(2pi) etc. is confusing to me.
  2. Saying that "we assume h=1" may not fully capture what's going on - at least it leaves room for confusion what that would imply for how to specify temperature/thermal energies k_B T. (As a separate aside, working in units where h=1 is not an assumption but a choice; we "set" h=1 is more appropriate than "assume".)

@petergthatsme: looking forward to your lead on this, and thanks @clarkmiyamoto for your patience as we discuss this. You picked an important point, and clarity on that is crucial!

@clarkmiyamoto
Copy link
Author

clarkmiyamoto commented Jul 19, 2023

No worries, thanks for getting back so quickly!

  1. Ah I meant to expose how $E_C / 2 \pi, E_J / 2\pi, \alpha / 2\pi$, etc. in literature usually implies linear frequency. But honestly, that notation isn't that clear ://.
  2. Ah sorry for the sloppy language, yes "set" is more appropriate. And as for thermal energies, I haven't had to work w/ those so I'll let someone else write about that.

I'll wait for Peter's feedback then make edits.

@petergthatsme
Copy link
Collaborator

Thanks for for your pull request @clarkmiyamoto and my apologies for such a slow response.

Regarding items above:

  1. Maybe as Jens suggests, you can get rid of the $E_C/2\pi$, etc. comment (I do know what you're saying, but in case it's confusing to some).
  2. yeah, saying "we set h=1" seems fine to me... I think users pass temperatures in Kelvin to various methods, so k_B T is not something they have to calculate explicitly themselves.

Anyway, if you don't mind, please make the changes above, and we can merge.

Thank you again for your input and help with your pull request!

@clarkmiyamoto
Copy link
Author

Hi @jkochNU @petergthatsme, apologies for the slow response time. I just started grad school and haven't been coding as much! I just made a push w/ your feedback. Let me know if you folks have any other comments/concerns.

@jkochNU
Copy link
Member

jkochNU commented Jan 8, 2025

@petergthatsme Can you let me know the status of this and whether it should be merged at this point?

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.

3 participants