QuantumBFS / Yao.jl

Extensible, Efficient Quantum Algorithm Design for Humans.
https://yaoquantum.org
Other
910 stars 118 forks source link

Fix issue 508 #509

Closed GiggleLiu closed 1 month ago

GiggleLiu commented 2 months ago

fix #508 by setting a looser threshold.

Roger-luo commented 2 months ago

I don't like this change, this is too heurestic, we should really just let user decide what precision they want. And only guarantee on throw or nothrow behaviour.

jlbosse commented 2 months ago

In what case would the proposed behaviour not be desirable? Grepping through the code safe_real(x::T) is currently only used in situations where we would expect x to be real up to precision eps(T) since it is the output of sum large-ish calculation that would be exactly real if we had no floating point error.

Its behaviour is i) return its input if x is real, ii) return the real part of x if the imaginary part is zero up to eps(T) if eps(T) is defined and iii) return the real part of x if the imaginary part is exactly zero if eps(T) is not defined. To me this is (with the exception of maybe iii) ) exactly the behaviour I would find sensible. And iii) would fail loudly if we get something with tiny, but non-zero imaginary part.

GiggleLiu commented 2 months ago

The only issue is, the eps, which is defined by the gap between two floating point numbers, might be a bit too strict. What about using sqrt(eps(T)) or 10 * eps(T)? I am fine to make it a keyword argument so that users can tune it when the program errors unexpectedly.

jlbosse commented 2 months ago

I personally would be happy with either option, I guess do whichever is more standard when doing numerical computations?

Roger-luo commented 2 months ago

The problem is we cannot know how large this eps means fine. This is exactly why you have rtol and atol in isapprox to control such with a good default if you want an approximate behaviour. What I'm proposing is either:

a) forward these two options in expect;

expect(...; atol=..., rtol=...)

b) if user turns on nothrow=True, fallback to the unsafe real conversion and let users decide what they want to do with the result

expect(...; nothrow=True) # this is explicitly saying the calculation will not throw and may not be safe

@jlbosse for #508 , it is exactly the case of an unexpected throw when someone expects it to go through by just ignoring the imaginary part despite of how large it is.

GiggleLiu commented 2 months ago

Ok, I will assume users know what they are doing. I will throw a warning for large floating point errors to let user know they might want to use sandwich function instead. Then expect will never throw error.

@Roger-luo I think the interface that you proposed is too complicated. When the inputs are non-floating point types, it does not make sense to set eps.

Roger-luo commented 2 months ago

I will throw a warning for large floating point errors to let user know they might want to use sandwich function instead. Then expect will never throw error.

that sounds good to me.