Closed liu-zixiong closed 9 months ago
Some intitial thoughts as I read through the suggestions the first time:
I pushed a docuemntation change for 2. For 4., I do see that we're using the bosonic-qiskit custom ParameterizedUnitaryGate. Since we're not trying to parameterize the matrix and we're already checking if it is unitary, could we instead follow the standard Qiskit way by using the UnitaryGate? https://docs.quantum.ibm.com/api/qiskit/qiskit.circuit.library.UnitaryGate I think then the disretization code should just send the gate as all one step without discretizing it at all.
In re-reading 5 -- ffmpeg is a software package installed on the system, it isn't a Python package, so it won't be in requirements.txt (I wasn't sure if you meant you installed the ffmpeg software separately or if you installed the ffmpeg named Python package). I've been able to get animate_wigner() working on Linux & Windows systems, and the tests are also running on MacOS in github. MacOS, Linux, and Windows are all tested on Github for Python 3.8, 3.9, 3.10, and 3.11, so it ought to work, but do please send your stack trace & errors my way. I'd defintely like to help get it working for you.
1, 2, and 4 are now addressed. The tutorials/README.md was updated or 3., but not to include full descriptions of each tutorial. The ffmpeg software is a system install, as far as I'm aware, there isn't a Python package that can be installed to include it for 5.
Hi everyone, happy new year! Over the past year working in this repo, there are a list of things which I've picked out which might be helpful to implement.
c2qa.util.avg_photon_num()
function only works with single qumodeThe docstring for
avg_photon_num()
does not yet mention that it's only designed to work with a single qumode. Attempts to use it with multiple qumodes/qumode registers will produce erroneous outputs.It would be convenient to have a function that takes in a state and a qumode of interest and returns the state for just that particular qumode. Additionally, it would also be helpful to rework
avg_photon_num
so that it returns a proper average across multiple qumodes.The
c2qa.util.simulate()
docstring needs to be updated. It is currently missing an explanation for certain args like "per_shot_state_vector" or "max_parallel_threads".It might be helpful to expand the
README.md
file in the tutorials folder so that there is an explanation for what each of the tutorials are. This is just so that it's easier to figure out what the tutorials are meant for, without needing to manually open every tutorial notebook. Alternatively, addingREADME.md
files to every subfolder would also achieve the same purpose (this would also make it easier to track which tutorials require READMEs, as and when they're added).I'm not sure if the compatibility between
c2qa.circuit.cv_gate_from_matrix()
anddiscretize
has ever been tested. This is not a super important becausecv_gate_from_matrix()
allows you to define physically unimplementable gates, and applyingdiscretize
to those gates wouldn't make sense anyways. But in the event that a user actually does define a physically implementable gate, it may be helpful to check thatdiscretize
actually produces the correct result in those cases.I've never managed to get
c2qa.animate.animate_wigner()
working. I've tried installing theffmpeg
package since it is mentioned to be a requirement in the docstring forc2qa.animate.animate_wigner()
(even though it wasn't included inrequirements.txt
), but I suspect I'm still missing a few key package installs. Other new users will likely also face trouble runningc2qa.animate.animate_wigner()
, especially if they are installing Bosonic Qiskit in a new virtual environment. It might be helpful to add the packages needed forc2qa.animate.animate_wigner()
in a separaterequirements-optional.txt
.Hope this helps! And thank you for the opportunity to work on this project, I had a lot of fun contributing new features.