Add unit tests for encoding module
#274
Conversation
There was a problem hiding this comment.
Hello! Discussed with the team and added a few more comments.
Essentially, we’re looking for more tests that check the functionality, so comparing the encoders against either hardcoded expected results or against an existing well-tested function from another library. It would be great if you could look into that for the encoders (as I believe you did for the StateEncoder).
Thanks so much for your hard work!
| qdev = QuantumDevice(2) | ||
| with mock.patch.object(encoder, "func") as mock_func: | ||
| encoder(qdev, torch.rand(2, 4)) | ||
| assert mock_func.call_count >= 1 |
There was a problem hiding this comment.
I’m unsure if this is necessary; what exactly are you checking here?
There was a problem hiding this comment.
I am mocking the func input here. Since this is a unit test for encoders, I need to isolate the functionality of the encoder regardless of the behaviour displayed by func.
func under the hood is a unitary gate. Unit testing for any gate input as func should be done in a separate file, just like how tests/operators/Controlled_Unitary is tested.
So you test the func separately + test the encoder separately = and then this indicates that they together work as you want.
| "batch_size, wires, funcs", | ||
| [(2, 5, ["ry", "phaseshift"]), (1, 4, ["u2"]), (3, 1, ["u3"])], | ||
| ) | ||
| def test_phase_encoding(self, batch_size, wires, funcs): |
There was a problem hiding this comment.
Similar to the StateEncoder, if you could develop the tests to verify the answer is correct (so compare the encoding to some baselines) for each of the encoders. These can either be hardcoded expected answers or compare them to the results of some library.
If you can do this for all of the encoders, that would be amazing!
There was a problem hiding this comment.
I will definitely look into it.
PS: Are you thinking along the lines of an integration test? I have never written one but I will try!
There was a problem hiding this comment.
Just treat it like writing some unit tests; if you need a reference, you can check out the other tests.
| """ | ||
| """ | ||
| # Validate inputs | ||
| self.validate_inputs(qdev, x) |
There was a problem hiding this comment.
Want to check if we want this @Hanrui-Wang. The pro is that it’s safer from errors but also could result in some extra overhead.
|
@SaashaJoshi Could you update this by the end of today to complete the bounty? Feel free to ask any questions if needed! |
|
I'll try my best to finish it today. I am on a little time crunch. |
Adding unit tests for
- [ ] Magnitude encoderAddresses #261