Added human-readable file IO for SymbolicOperators #311
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
|
I signed it! |
|
CLAs look good, thanks! |
| @@ -396,13 +396,15 @@ def inverse_fourier_transform(hamiltonian, grid, spinless): | |||
| vec_func_2=momentum_vector) | |||
|
|
|||
|
|
|||
| def load_operator(file_name=None, data_directory=None): | |||
| def load_operator(file_name=None, data_directory=None, human_readable=False): | |||
There was a problem hiding this comment.
Instead of human_readable let's call this plain_text.
| """Load FermionOperator or QubitOperator from file. | ||
|
|
||
| Args: | ||
| file_name: The name of the saved file. | ||
| data_directory: Optional data directory to change from default data | ||
| directory specified in config file. | ||
| human_readable: Whether the operator should be loaded from a | ||
| human-readable format |
There was a problem hiding this comment.
"Whether the input file is plain text."
| loaded_fermion_operator = load_operator(self.file_name, | ||
| human_readable=True) | ||
| self.assertEqual(self.fermion_operator.terms, | ||
| loaded_fermion_operator.terms) |
There was a problem hiding this comment.
self.assertEqual(self.fermion_operator, loaded_fermion_operator)
There was a problem hiding this comment.
Should the rest of the test cases be comparing the original operator and the loaded operator in this way? Including the cases that have already been written?
There was a problem hiding this comment.
That would be nice but I won't require it. Those tests were actually written before this was possible; we didn't always implement __eq__ in SymbolicOperator.
| map(complex, tm.values())))), f) | ||
| if human_readable: | ||
| with open(file_path, 'w') as f: | ||
| f.write(operator_type + ": " + str(operator)) |
There was a problem hiding this comment.
I think we'll want to do :\n instead of : . Can you add a test with small operator that explicitly includes what the saved format looks like? I.e., the test should compare the saved file with an explicit string.
| for term in operator_terms: | ||
| operator += QubitOperator(term, operator_terms[term]) | ||
| else: | ||
| raise TypeError('Operator of invalid type.') |
There was a problem hiding this comment.
It should not be necessary to add terms individually. It should be possible to simply do something like operator = FermionOperator(some_string) where some_string is probably going to be straight from the file, minus the initial heading which says what type it is.
There was a problem hiding this comment.
I think this code is constructing the operators as you suggest. It uses the split function only to separate the type of the operator from the remaining data, which is stored as a string when the data is loaded from plain text
There was a problem hiding this comment.
Ah, I was looking at the wrong part of the code. Yes, you did do it that way. Great!
|
Thanks for the contribution! Feel free to add your name and affiliation to the NOTICE and README, if you like. |
|
... in the alphabetical section, please! |
* Added human-readable file IO for SymbolicOperators * Renamed human_readable to plain_text and fixed tests * Changed operator equality tests to use __eq__ instead of directly comparing terms
Proposed resolution to #122