added free projection without open ended walks #291
Conversation
There was a problem hiding this comment.
Thanks for your contribution! This is great. There are a few things which aren't necessarily your problem but a failure of design / documentation on our part:
I think big features like this (and @linusjoonho can comment since his group is really actively developing features) should likely live in examples entirely and be structured in a minimal set of files (driver, propagator, walker, ... unit tests etc) and just import the core ipie features used. Most of my comments are related to this, and would be solved by writing something more stand-aloney. python is very flexible so we should exploit that.
It would also be great to demonstrate that the code is working by adding maybe a short notebook demonstrating that FP is getting the right answer and even better if ph-AFQMC is not.
|
thanks for the feedback! because the fp afqmc I implemented is adjacent to but not exactly the same as phaseless, it is a bit tricky to design. I have completely severed it from the phaseless code now, so it can be moved to a completely different place as well. |
ipie/analysis/extraction.py
Outdated
| @@ -68,6 +68,49 @@ def extract_observable(filename, name="energy", block_idx=1): | |||
| return walker_averaged.reshape((nsamp,) + tuple(obs_info["shape"])) | |||
|
|
|||
|
|
|||
| def extract_hdf5_data_complex(filename, block_idx=1): | |||
There was a problem hiding this comment.
Looks identical to extract_hdf5_data. Is it any different from extract_hdf5_data? Why couldn't you re-use extract_hdf5_data?
There was a problem hiding this comment.
the other function writes only real values
There was a problem hiding this comment.
which function? I'm pretty sure we write estimators as complex values by default, so I'm still confused why we need both?
There was a problem hiding this comment.
There was a problem hiding this comment.
Wait a second - why do we need to use hdf5 data at all? We are discontinuing the support for hdf5 output, aren't we?
There was a problem hiding this comment.
I would vote against discontinuing hdf5 output:
- We serialize the AFQMC object which stores useful information (maybe this is not that useful, but still worth thinking about)
- We can store more complex observables (multi-dimensional arrays) more easily.
- It's descriptive.
https://github.com/JoonhoLee-Group/ipie/blob/develop/ipie/analysis/extraction.py#L33 ?
Ah, right. I think what you've done is ok then for the moment, a better fix might be to handle this AFTER extraction since the complex valued data has more information.
|
LGTM |
No description provided.