Closed alankao64 closed 5 years ago
Added assert calls in read_circuit for the following functions that weren't tested: fSim() and gate_arrays()
Questions: Should we test if an ordering is valid every time we pass an ordering into a function? Do the current error messages for nullptr failures need to be more specific?
Should we test if an ordering is valid every time we pass an ordering into a function?
Checking once (likely when the ordering is first read from the file) should be sufficient.
Hm ok, so should I call IsOrderingValid() in evaluate_circuit.cpp when the ordering is generated (line 88)? It might not be necessary because later in the EvaluateCircuit() function, when ContractGrid() is called, one of the first checks it does is to call IsOrderingValid() on the ordering that is passed in.
Hm ok, so should I call IsOrderingValid() in evaluate_circuit.cpp when the ordering is generated (line 88)? It might not be necessary because later in the EvaluateCircuit() function, when ContractGrid() is called, one of the first checks it does is to call IsOrderingValid() on the ordering that is passed in.
That seems reasonable. Probably the cleanest solution is to call IsOrderingValid right at the end of ordering_data_to_contraction_ordering - that way we can safely assume that orderings elsewhere in the code are valid (and remove the extra IsOrderingValid calls).
That seems reasonable. Probably the cleanest solution is to call IsOrderingValid right at the end of ordering_data_to_contraction_ordering - that way we can safely assume that orderings elsewhere in the code are valid (and remove the extra IsOrderingValid calls).
Ok, the only IsOrderingValid call now is at the end of ordering_data_to_contraction_ordering(). I'm struggling to come up with a way to test this. Is there a variation of kSimpleOrdering[] (found in contraction_utils_test.cpp) that I can pass in to generate an invalid ordering?
Is there a variation of kSimpleOrdering[] (found in contraction_utils_test.cpp) that I can pass in to generate an invalid ordering?
This will generate a similar ordering to the one previously used in the "ContractGridInvalidInput" test:
constexpr char kInvalidOrdering[] = R"(# test comment expand a 1 expand a 1 )");
If you need to write something more complex, check docs/input_format.md
or any of the sample orderings in the ordering
directory for examples.
Thanks! I think this PR is almost good to go. For the nullptr handling in tensor.cpp, I haven't tested the following "private" functions yet: Tensor::_fast_reorder, Tensor::_left_reorder, Tensor::_naive_reorder, _multiply_MM, _multiply_Mv, _multiply_vM, _multiply_vv. I'm going to see if I can by calling other functions, but I'm not sure if we need to test the multiply functions.
For both the reorder and multiply functions, the public methods already check the pointers for null - no additional tests should be necessary.
Adding asserts and tests for places where the possibility of a nullptr being passed into a function exists.