ubsuny / 23-Homework1G1

Homework 1 Repository Group 1
0 stars 6 forks source link

The code does not multiply "large" integers #40

Closed AhmedCode99 closed 12 months ago

AhmedCode99 commented 12 months ago

During unit testing, it was shown that the code returns false values when multiplying larger numbers. For example, multiplying 10 by 1000 returns 31. Any ideas on why that is happening and how to fix it?

WildJimmy commented 12 months ago

Hi Ahmed,

The program only works for integers 5 bits or less (so less than or equal to 16). I kept it this way to try to avoid the huge calculation times than can come with many qubits. To fix this, you would just have to increase the register size and make sure the qubits are properly encoded. Here's a more in depth explanation of what I mean:

q = qiskit.QuantumRegister(19) c = qiskit.ClassicalRegister(9) This line intializes quantum and classical registers where we can store the information. The quantum register is used for the circuit and manipulations, and the classical register is only needed for the final answer. The arguments are the number of bits in the register. It's important to notice that the quantum register needs to store 2 numbers to multiply and the final result, so the register needs space for all three. I.e. for 2 5-bit numbers we need 5 bits for the multiplier, 5 for the multiplicand, and then 9 for the answer for a total of 15. (9 for the answer because the greatest possible result is (16)(16) = 256 = 9 bits.) I think this generalizes to needing a quantum register of size 4(number of bits in larger number) - 1, which could be configured by setting a new variable like maxBitSize = max(len(bin(multiplier)), len(bin(multiplicand))) - 2 qRegSize = 4*(maxBitSize) -1 Where we get the number of bits by taking the length of the binary representation of the larger number (-2 because python represents them with an 0b in front. Kind of annoying.) And then make the register around this. Both integers must have the same number of bits in the register which is why we make space for both based on the larger of the two.

The classical register would then just be something like cRegSize = 2*(maxBitSize) -1 In the same vein as the prior one. Then we can just replace our prior definitions with q = qiskit.QuantumRegister(qRegSize) c = qiskit.ClassicalRegister(cRegSize)

That is the majority of the issue. The only place to be careful is that this value would need to be referenced elsewhere. This value comes up in the following places:

Line 32: `for i in range(multiplierBinLen):

print(int(bin(multiplier)[len(bin(multiplier))-1-i]))

if int(bin(multiplier)[len(bin(multiplier))-1-i]) == 1:
  circuit.x(q[i + 5])`

(Please ignore the commented out print statement that I just realized I left in) -Specifically, the final line should now read circuit.x(q[i + maxBitSize]) So that we properly allocate space if the first number is not the larger of the two (I can explain this more if need be.)

Line 38: qft_circuit = qiskit.circuit.library.RGQFTMultiplier(num_state_qubits=5, num_result_qubits=9) -This can now simply be qft_circuit = qiskit.circuit.library.RGQFTMultiplier(num_state_qubits=maxBitSize, num_result_qubits=cRegSize) Since we have defined specific variables for these quantities now

Line 42: circuit.measure(q[10:19],c) -We now need to ensure that we're only measuring the qubits that store the result, so we'll generalize this to circuit.measure(q[(qRegSize-cRegSize):qRegSize],c) Which is a little messy, but should give only the last (however many bits the result is stored in) qubits to be measured

Okay, that's a lot I know but hopefully it should clear it up. I think I will try these changes on my machine but I will let you also check and contribute as part of your unit test to ensure you get credit. Let me know if you need anything cleared up!

Edited to fix some formatting problems

WildJimmy commented 12 months ago

@uarif This may also be helpful for your documentation!

WildJimmy commented 12 months ago

@AhmedCode99 So I just tested it and these changes do work, with two caveats:

-It does take a long time to do larger numbers. I tested a bunch of smaller numbers and now I'm still waiting for 100100 -For larger numbers (i.e. 200200) I'm told there isn't enough memory. (I'm using the IBM quantum backend.) I don't think there's much we can do about this and it's just a result of the limitations of using quantum computing like this. Before if you tried to multiply 1000*10 you just got an incorrect number because the register size was fixed. I.e. It "rewrote" parts of the register because there weren't enough bits to properly store it. Now it will try to allocate the register space but be honest and say that it doesn't have the capacity to run that many qubits. That's what I think at least

WildJimmy commented 12 months ago

Sorry for all the comments, but one more discovery:

-I made a slight error with the register sizes, they should be qRegSize = 4*(maxBitSize) cRegSize = 2*(maxBitSize) I was trying to be clever and conserve space and realized when testing that it should of course be 2n bits to store the result, because for example 15*15 = 225 15 is 4 bits 225 is 8 bits

I don't know what I was thinking initially, but this should fix it I think

WildJimmy commented 12 months ago

@AhmedCode99 just saw that you are already done with the unit test, so no worries I will implement these

WildJimmy commented 12 months ago

Changes are implemented on main, should comply with all unit tests (barring those where the memory needed is too large)