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

Dev/keldysh dlr new imaginary time Green's function type with DLR representation using Lehmann.jl #8

Merged
merged 8 commits into from
Jul 24, 2024

Conversation

HugoStrand
Copy link
Collaborator

Dear Igor,

This is pull request that adds a new imaginary time Green's function type using the Discrete Lehmann Representation and Lehmann.jl. This enable us to represent hybridization functions at a desired accuracy using very few coefficients, and enabling arbitrary interpolation by evaluating a few exponential functions.

The feature is "opt in" in the sense that the user can construct hybridization functions of DLRImaginaryTimeGF type instead of the standard equidistant ImaginaryTimeGF and pass them on to QInchworm.jl.

@HugoStrand HugoStrand requested a review from krivenko July 11, 2024 19:55
@krivenko
Copy link
Owner

Hey Hugo,

A quick question before I do a proper review. Do you insist on using your own DOS integration routine (semi_circular_g_tau()) in test/keldysh_dlr.jl instead of Keldysh.dos2gf()?

@HugoStrand
Copy link
Collaborator Author

Dear Igor, No I do not insist. Please let me have a go at removing that (and the QuadGK dependence). Thank you for asking.

@krivenko
Copy link
Owner

Please let me have a go at removing that (and the QuadGK dependence).

No need to. I have already removed it but wanted to ask you before commiting.

Copy link
Owner

@krivenko krivenko left a comment

Choose a reason for hiding this comment

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

I generally try to follow the Blue Style Guide for Julia. Let us stick to the 92 character line length limit it recommends.

test/bethe_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
@HugoStrand HugoStrand requested a review from krivenko July 15, 2024 21:10
src/keldysh_dlr.jl Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
src/keldysh_dlr.jl Outdated Show resolved Hide resolved
@HugoStrand
Copy link
Collaborator Author

Dear Igor,

Thank you for the 2nd round of feedback. For some reason I can not reply directly under some of your comments. I have adopted all your suggestions and added tests for same statistics in the Green's function arithmetic operations involving two Green's functions.

Please have a look when you are back from vacation.

Cheers, Hugo

@krivenko
Copy link
Owner

Hey Hugo,

For some reason I can not reply directly under some of your comments.

This might have something to do with those comments being non-review comments...

Please have a look when you are back from vacation.

I'd say this PR is ready to be merged today provided you resolve the last remaining issue: #8 (comment)

@HugoStrand
Copy link
Collaborator Author

As per in person discussion I am merging this. Thank you for the help @krivenko!

@HugoStrand HugoStrand merged commit 3df3a2e into main Jul 24, 2024
5 checks passed
@krivenko krivenko deleted the dev/keldysh_dlr branch July 24, 2024 13:37
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.

2 participants