-
Notifications
You must be signed in to change notification settings - Fork 30
Make variable names more verbose and remove comments #926
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
Conversation
qupulse/utils/__init__.py
Outdated
| rounded_up_duration = sp.ceiling(duration / duration_per_quantum) * duration_per_quantum | ||
|
|
||
| next_multiple_sp = sp.Piecewise( | ||
| (0, sp.Le(duration, 0)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I compare the duration here directly. Is it faster to compare the integers? I did find a difference in my benchmarks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(see comment in #925 )
Nomos11
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can change the piecewise line if it's syntactically nicer, but for me it was slower in %timeit
|
Oh, I don't think it is syntactically nicer. Just a bit shorter because the quanta do not need to be stored. The important part to me is to avoid non-descriptive variable names unless the meaning is clear in a given domain (loop indices, standardized physical or math notation) |
|
completely agree with the variables; just meant the change from int to float in sp.Le which - it's probably that? - for me made it slower. |
|
that wouldn't make much sense however since that's on evaluation and the instantiation was slower, so idk... |
…based on the quanta
No description provided.