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

adding ElementCustomIonPotential + new GP example using it (with doc)… #251

Merged
merged 6 commits into from
Jun 5, 2020

Conversation

gkemlin
Copy link
Collaborator

@gkemlin gkemlin commented Jun 5, 2020

  • new element for to generate AtomicLocal term with custom potential
  • new example of 1D GP with SCF and forces using ElementCustomIonPotential with docs
  • small typo in 2D GP example
  • modified Model.jl to detect 1D/2D models and automatically disable symmetry

docs/src/index.md Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
src/Model.jl Outdated Show resolved Hide resolved
src/elements.jl Outdated Show resolved Hide resolved
src/elements.jl Outdated Show resolved Hide resolved
src/elements.jl Outdated Show resolved Hide resolved
src/elements.jl Outdated Show resolved Hide resolved
@antoine-levitt
Copy link
Member

Also should that custom type be in the example file rather than in elements.jl? I think it makes sense since it's so simple. Also if in the DFTK source it poses the question of should Coulomb be a particular case of this, which I'm not sure we want to bother with.

@gkemlin
Copy link
Collaborator Author

gkemlin commented Jun 5, 2020

I think that it's only 18 lines of code at the end of the element.jl file so having it implemented in the code or in an example doesn't change that much. In the former case, users only have to call ElementCustomIonPotential(Z, pot_real, pot_fourier) to implement their potential without thinking about this Element structure and it might be a good thing. But if you prefer the other way round, it's good for me.
Keeping Coulomb as a particular Element might be better ? This custom type is more for "exploration" whereas Coulomb has real interest in practice.

@antoine-levitt
Copy link
Member

I think it's better outside: better to have a small code base that makes simple extensions easy than have all the extensions in the code.

Copy link
Member

@mfherbst mfherbst left a comment

Choose a reason for hiding this comment

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

Nice example. Yes I agree no need to put this into the main code. Maybe at some point when we find we need it more often.

src/Model.jl Outdated Show resolved Hide resolved
docs/src/index.md Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
examples/gross_pitaevskii_custom_potential.jl Outdated Show resolved Hide resolved
L = 0.5;
# We set the potential in its real and Fourier forms
pot_real(x) = exp(-(x/L)^2)
pot_fourier(q::T) where {T <: Real} = exp(- (q*L)^2 / 4);
# And finally we build the elements and set their positions in the `atoms`
# array. Not that in this exemple, `pot_real` is not necessary as all application
# array. Note that, in this example, `pot_real` is not required as all applications
Copy link
Member

Choose a reason for hiding this comment

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

No comma after that.

@mfherbst mfherbst merged commit a765309 into master Jun 5, 2020
@mfherbst mfherbst deleted the forces_GP_1D branch June 5, 2020 17:51
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.

3 participants