Skip to content
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

Avoid statevector copy in qsim.simulate(qc, initial_state=sv) #617

Open
hthayko opened this issue Jun 24, 2023 · 12 comments
Open

Avoid statevector copy in qsim.simulate(qc, initial_state=sv) #617

hthayko opened this issue Jun 24, 2023 · 12 comments

Comments

@hthayko
Copy link

hthayko commented Jun 24, 2023

I have a use case where I am simulating my circuit piece by piece so using the ability to pass an initial state to qsim is very handy.
However I have noticed this makes a copy of the initial_state here:

Is there a way to avoid the expensive copy?
If there is a little patch that can be applied (I assume couple of lines in void init_state() should do it) I would be happy to apply it locally and re-build qsim.

Thanks!

@95-martin-orion
Copy link
Collaborator

Hello Hayk! As I recall, that copy step exists to pass ownership of the state from Python to C++. Copyless transfer of ownership in the opposite direction is possible with pybind (and we in fact do so in this method), but it's less clear if Python-to-C++ passing can be done without copying. There's some discussion of it in this pybind issue, but it remains unresolved.

I'm no longer actively working on qsim, but @pavoljuhas or @sergeisakov might be available to look into this. Otherwise, if you're interested in pursuing this yourself you could look into the pybind issue above - I'd be more than happy to review a PR if you discover a way to avoid this copy 🙂

@sergeisakov
Copy link
Collaborator

In principle, we don't have to transfer ownership if there is a guarantee that the state doesn't get garbage collected by Python during simulation. Though there might be another problem. State memory must be aligned on a 32-byte or 64-byte boundary when using AVX or AVX512.

@hthayko
Copy link
Author

hthayko commented Jun 26, 2023

I tried this little fix, it works fine sometimes but more often than not it gives a segmentation fault:

   void init_state(const py::array_t<float> &input_vector) {
     StateSpace state_space = factory.CreateStateSpace();
     py::buffer_info buf_info = input_vector.request();
     float* ptr = static_cast<float*>(buf_info.ptr);
     state = state_space.Create(ptr, state.num_qubits());
     state_space.NormalToInternalOrder(state);
   }

Seems like the main issue is the python garbage collector.
This is needed for a one-off job I have so I would be happy with a hacky solution as well if you can think of one (disabling gc temporarily, etc.)

@pavoljuhas
Copy link
Collaborator

Seems like the main issue is the python garbage collector.

The input_vector array will not get garbage collected if it it is referenced by some variable in a Python caller of init_state.

@hthayko
Copy link
Author

hthayko commented Jun 28, 2023

I have all the variables in global scope, and the reference is kept - but still getting the segmentation fault.

import cirq
import qsimcirq

def get_all_H(N):
	circuit = cirq.Circuit()
	qubits = cirq.LineQubit.range(N)
	circuit.append([cirq.H(qubits[i]) for i in range(N)])
	return circuit

qc1 = get_all_H(10)

simulator = qsimcirq.QSimSimulator({'t': 8, 'f': 3, })
results0 = simulator.simulate(qc1)
results1 = simulator.simulate(qc1, initial_state=results0.final_state_vector)

Here the last line sometimes succeeds and sometimes segfaults.

@sergeisakov
Copy link
Collaborator

There are two issues here. First, [](void *data) { detail::free(data); } shouldn't be used in line 788 (pybind_main.cpp) in order to avoid double free. Second, the state vector allocated in Python must be aligned. Typically this is not the case.

@hthayko
Copy link
Author

hthayko commented Jun 28, 2023

hmmm - the free at pybind_main.cpp:788 releases the state back to python. Where is the second (double) free?

@sergeisakov
Copy link
Collaborator

The free at pybind_main.cpp:788 deallocates the memory allocated on the C++ side. If the memory is allocated by Python then this free tries to perform additional deallocation.

@hthayko
Copy link
Author

hthayko commented Jun 29, 2023

In that case there should be an issue on the second deallocation, e.g. when python gc kicks in after global scope variables are cleaned up. However I see seg fault before that - if I print results1.final_state_vector in my python code above - it will work half the time and segfault the other half.

@sergeisakov
Copy link
Collaborator

There is also a memory alignment issue. You see seg fault before printing when the state vector is not properly aligned.

@hthayko
Copy link
Author

hthayko commented Jul 1, 2023

that issue should be deterministic though right? However I see the same code working ~20% of the time and seg faulting 80% of the time.

@sergeisakov
Copy link
Collaborator

I think it shouldn't be deterministic. You can try to add printf("%lu\n", (uint64_t) ptr % 32) or printf("%lu\n", (uint64_t) ptr % 64) (depending on what you have: AVX or AVX512) to your little fix. If it prints 0 then the memory alignment is okay and there shouldn't be a seg fault before memory deallocation.

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

No branches or pull requests

4 participants