Closed andreamari closed 3 years ago
Hey, i'll like to work on this issue
@andreamari I've made the suggested changes and created a pull request, you can review it now.
Should we round it to the nearest odd integer instead of raising an error @andreamari?
Should we round it to the nearest odd integer instead of raising an error @andreamari?
Actually, the rounding that you suggest is what the function is already doing.
Maybe we can only raise a warning (instead of an error) if the scale_factor
is not np.isclose
to its odd integer approximation.
My only concern is that the user may not be aware of the such a crude rounding approximation. E.g. scale_factor = 2
is rounded to 1
.
I'm ok with a warning but I don't think its necessary as long as the function is well-documented and clearly states it will round the scale factor.
I think raising an error is bad behavior though.
Users may be tempted to use this function: https://github.com/unitaryfund/mitiq/blob/2cb68d64a0f2ebfc98f6c3fbb03cb9c67f516cdf/mitiq/zne/scaling/folding.py#L240
with arbitrary scale factors. However this function, by construction, makes sense only if the scale factor is an odd integer. We may sill keep the type of
scale_factor
asfloat
for consistency with other methods, but we should check ifscale_factor
is approximately equal to an odd integer. If not, we can raise aValueError
.