Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gf] GfReFreq density density_zero_T + test #534

Merged
merged 4 commits into from
Nov 30, 2018

Conversation

HugoStrand
Copy link
Member

This is a remake of pull-request #391 with added text and finite temperature functionality.

Contains .density(beta) and .density_zero_T() functions for GfReFreq.

@HugoStrand
Copy link
Member Author

This was more than six months ago (@mzingl started this two years ago). It has the test asked for in #391. Best, Hugo

@parcollet parcollet self-assigned this Nov 14, 2018
@parcollet parcollet self-requested a review November 15, 2018 02:45
Copy link
Member

@parcollet parcollet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1- Do we really want density and density_zero_T ?
Why not an overload density(beta) and density() ?
Is it clear enough ? (@Wentzell : Opinion on just API requested here).

2- total_density : why do we provide this ?
Taking a trace is trivial (both C++ and numpy) , let's not multiply functions with low contents in the API
it inflate the library for nothing.

@mzingl
Copy link
Member

mzingl commented Nov 15, 2018

To
1.) My opinion is that density() should give the density at T=0. So the default density for GfReFreq should be without Fermi function.
2.) Removing it will brake backward compatibility, or? I guess many are using it in DMFT scripts, at least I do.

@Wentzell
Copy link
Member

Why not an overload density(beta) and density() ?
Is it clear enough ?

As long as this is clearly stated in the documentation I am fine with this API.

2- total_density : why do we provide this ?

I agree with Manuel that this should be kept for backward compatibility.

@HugoStrand
Copy link
Member Author

I have tried to rebase this and fix the comments. But, since I wrote this in February... the whole structure of the python bindings of the greens function has changed.

The files gf_desc.py, functions.cpp and functions.hpp do no longer exist, and the structure in the python bindings seems no longer to add these functions as methods to the Greens function class but rather as free functions.

@Wentzell how and where should the wrapping be done? I am not keen on adding this to gf_fnt.py see #635.

Best, H

@parcollet
Copy link
Member

Ok, we will update the wrappers.

@Wentzell
Copy link
Member

@Wentzell how and where should the wrapping be done? I am not keen on adding this to gf_fnt.py see #635.

At the moment the file to add them to is gf_fnt.py, but I agree that the file should be split.

@HugoStrand
Copy link
Member Author

I have rebased on unstable, moving the implementation to gfs/functions/density.* and put the wrappers in gf_fnt.py and removed the zero_T methods.

To use the suggested overloading where G.density() is zero temperature and G.density(beta) is finite temperature I had to add *args to the density and total_density methods in gf.py and block_gf.py.

With that I consider all comments accounted for.

@HugoStrand HugoStrand changed the base branch from master to unstable November 16, 2018 16:10
@HugoStrand
Copy link
Member Author

This is pending code review. From @Wentzell or @parcollet .


assert(wmin < 0.);

int N0 = floor(-wmin / dw) + 1; // frequency index at or above w=0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::floor !!


// -- Required to filter out divergent real parts of g that are inf
// -- eg flat dos at dos edge
for (int idx : range(0, res.shape()[0])) res(idx, idx) = dcomplex(0., imag(res(idx, idx)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// FIXME iterate on diagonal

// -- eg flat dos at dos edge
for (int idx : range(0, res.shape()[0])) res(idx, idx) = dcomplex(0., imag(res(idx, idx)));

res *= dcomplex(0., 1.) * g.mesh().delta() / M_PI; // scale to density
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1i ( standard complex literal !).
1_j

@HugoStrand
Copy link
Member Author

@parcollet this passed the CI, so I merge it.

@HugoStrand HugoStrand merged commit e21df64 into TRIQS:unstable Nov 30, 2018
@HugoStrand HugoStrand deleted the dev_gfrefreq_density branch December 6, 2018 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants