Ensure LRE compatibility with all supported frontends #2547
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2547 +/- ##
=======================================
Coverage 98.72% 98.72%
=======================================
Files 92 92
Lines 4161 4167 +6
=======================================
+ Hits 4108 4114 +6
Misses 53 53 ☔ View full report in Codecov by Sentry. |
mitiq/lre/tests/test_lre.py
Outdated
| ) | ||
|
|
||
| assert isinstance(lre_exp_val, float) | ||
| assert mock_executor.call_count == 10 |
There was a problem hiding this comment.
Unit tests are also good to undersrtand how the logic you are testing works (LRE in this case). Reading this, the immediate question I have is: why 10, how is it calculated? Proposal: add an explicit calculation, e.g. expected_call_count = degree * (degree + fold_multiplier) <-- I made this up, since I genuinely don't know where the 10 comes from.
There was a problem hiding this comment.
This still pending? It's not a big deal, but it makes me feel that the test expectation was written based on what result we empirically got by running the code, instead of the other way around.
There was a problem hiding this comment.
Ah sorry, I changed this here, but not here! I'll update.
|
I think I've resolved all the comments in c2eb37c. Apologies for not splitting it up into different commits, but everything got intertwined as I started working on it. |
mitiq/lre/tests/test_lre.py
Outdated
| ) | ||
|
|
||
| assert isinstance(lre_exp_val, float) | ||
| assert mock_executor.call_count == 10 |
There was a problem hiding this comment.
This still pending? It's not a big deal, but it makes me feel that the test expectation was written based on what result we empirically got by running the code, instead of the other way around.
Description
This PR ensures that the core functionality of LRE is compatible with all supported frontends.
fixes #2416