Detach prefix util function

[nkanazawa1989]

Summary

This is a counterpart of apply_prefix function. The new function returns a tuple of scaled value and prefix from a given float value. This function may be used in qiskit-experiment for unit handling (apply_prefix is already used in some places but we don't have its counterpart).

Details and comments

See unittests

[jakelishman]

This will emit a warning if value is 0 or less, though we probably still want to be able to handle them so they can be used with differences.

[nkanazawa1989]

Good catch! Thanks @jakelishman :)

[jakelishman]

Mostly just comments about particular exceptions we're throwing here, rather than anything else.

[jakelishman]

What's the use case for allowing ParameterExpression here? It seems more likely that the caller would be responsible for ensuring they're passing floats, rather than us.

[nkanazawa1989]

That is the best scenario, but we have one exception

delay = Parameter("delay")

qc = QuantumCircuit(1)
qc.delay(delay, 0, unit="us")

qc = transpile(qc, backend)

In this code, we parametrize the delay in units of us. The transpiler needs to convert the SI time unit into dt to submit this circuit to backend. First, it applies prefix, then it multiplies backend._configuration.dt. ParameterExpression supports simple multiplication, thus these operation should be done while the actual value is parametrized. Note that unbound parameter expression cannot be converted into float.

[nkanazawa1989]

Separate logic is removed with Decimal, but the import is still kept for type annotation.

[jakelishman]

The decimal form doesn't really avoid any more round-off errors than just doing it in floating point, if we take care to only work with positive powers of 10. All positive powers of 10 up to 10^22 have exact representations (float(1e23) differs from the exact value by 2^23), and since floating-point operations guarantee that the result will be the closest float to the infinite-precision answer, there's the same amount of rounding here (conversion to float at the end) as there would be if we did the previous multiply-or-divide trick to keep exact representations.

That said, I don't really mind one way or the other that much. I doubt (hope, even!) that this isn't performance-critical code (where avoiding Decimal would be better), and as you say, we'll never really be able to entirely sensibly handle cases such as apply_prefix(1e-7, 'µs'), since float('1e-7') < Decimal('1e-7'), but float('1e-1') > Decimal('1e-1').

[nkanazawa1989]

Umm... this is good point. If we avoid Decimal we don't need to handle exception for ParameterExpression. I'll try multiply-or-divide trick too.

[nkanazawa1989]

That worked. 1ba3e95

[jakelishman]

Seems fine now overall. Given that apply_prefix is already here, there's no harm in having its counterpart. There's always more that can be done with float handling, but if this is good enough for the use-cases that you have in mind, it can go in.

[mtreinish]

LGTM, just a couple questions inline.

[mtreinish]

What about µ here?

[nkanazawa1989]

This is caught by another test
https://github.com/Qiskit/qiskit-terra/blob/832ee84007e5d0752ca391911cfe960931032103/test/python/utils/test_units.py#L33-L34

[mtreinish]

Shouldn't we validate the returned prefix matches the input from detatch_prefix()? For u and µ we can just check that they both return µ.

[mtreinish]

LGTM, thanks for the quick update

[mtreinish]

I also think my earlier suggestion might have messed up the docs because that note ends up between Returns and Raises we'll have to see how it comes out and adjust accordingly

[nkanazawa1989]

Yeah, Google vs Numpy. I usually use google docstring stlye, but still fine with .. note::

[jakelishman]

Looks like the docs are all fixed now, so this should be good.