Skip to content

Conversation

@dwierichs
Copy link
Contributor

Context:
We often want to compute the integer $\lceil \log_2(n)\rceil$ of an integer $n$.
This currently requires three function calls if we want to receive an int as output: out = int(np.ceil(np.log2(n)))
In various places in the codebase, the call to int is actually skipped, so that variables that conceptually describe integers remain float values in code.

Description of the Change:
Add a convenience function qml.math.ceil_log2 that bundles the three function calls into one.

Benefits:
Declutter code base.
Make sure we are using int values where applicable.

Possible Drawbacks:
N/A

Related GitHub Issues:

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented Jan 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.44%. Comparing base (31f5441) to head (219bdd5).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8972   +/-   ##
=======================================
  Coverage   99.44%   99.44%           
=======================================
  Files         601      601           
  Lines       64754    64759    +5     
=======================================
+ Hits        64393    64398    +5     
  Misses        361      361           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@comp-phys-marc
Copy link
Contributor

This looks great! Only thing is I am still seeing 3 instances of math.ceil(math.log2(...)) in the codebase on your branch. Could you look into them please?
Screenshot 2026-01-23 at 9 56 19 AM

@dwierichs
Copy link
Contributor Author

This looks great! Only thing is I am still seeing 3 instances of math.ceil(math.log2(...)) in the codebase on your branch. Could you look into them please?

Oh thanks for catching those! The first two I overlooked because they are in tests/
The last one actually needs to stay because we do want the following to be supported:

n = 0
a = np.log2(n) # -np.inf
b = np.ceil(a) # -np.inf
c = 2**b # 0.0

I know, it's a bit of an unconventional computational path, but there actually is a test taking it (which is why I know, heh.)
I think I'll replace this, because we anyways replace 0.0 by 1 3 lines further down :D

@comp-phys-marc
Copy link
Contributor

This looks great! Only thing is I am still seeing 3 instances of math.ceil(math.log2(...)) in the codebase on your branch. Could you look into them please?

Oh thanks for catching those! The first two I overlooked because they are in tests/ The last one actually needs to stay because we do want the following to be supported:

n = 0
a = np.log2(n) # -np.inf
b = np.ceil(a) # -np.inf
c = 2**b # 0.0

I know, it's a bit of an unconventional computational path, but there actually is a test taking it (which is why I know, heh.) I think I'll replace this, because we anyways replace 0.0 by 1 3 lines further down :D

Glad to be of service. Sounds interesting, while I don't understand why we want to do that with the negative infinities etc.

@dwierichs
Copy link
Contributor Author

Glad to be of service. Sounds interesting, while I don't understand why we want to do that with the negative infinities etc.

Unfortunately I am missing the context here myself, but it seems to be a supported case that opt_width_continuous=0, which is then mapped to a lower bound of opt_width_continuous right after 🤷

@dwierichs dwierichs requested a review from andrijapau January 23, 2026 15:50
Copy link
Contributor

@comp-phys-marc comp-phys-marc left a comment

Choose a reason for hiding this comment

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

Looks great! Good QoL improvement.

Copy link
Contributor

@andrijapau andrijapau left a comment

Choose a reason for hiding this comment

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

Thanks 🚀 !

@dwierichs dwierichs added this pull request to the merge queue Jan 25, 2026
Merged via the queue into master with commit 245d425 Jan 25, 2026
55 checks passed
@dwierichs dwierichs deleted the ceil-log2 branch January 25, 2026 13:52
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