Refactor: round 2 #4
Conversation
|
@Br0kenSmi1e Why the radius of Ti so small?
|
GiggleLiu
left a comment
GiggleLiu
left a comment
There was a problem hiding this comment.
@Br0kenSmi1e I need your help to understand some parameters.
| - `population_list::AbstractVector{Int}`: The number of atoms of each species. | ||
| - `interaction_matrix::AbstractMatrix{T}`: The interaction matrix. | ||
| - `optimizer`: The optimizer. | ||
| - `optimizer_options`: The options for the optimizer, e.g. `optimizer_options = Dict("NodefileSave" => 1)` for Gurobi. |
There was a problem hiding this comment.
What is this NodefileSave for?
There was a problem hiding this comment.
It is an advanced parameter of Gurobi solver. https://docs.gurobi.com/projects/optimizer/en/current/reference/parameters.html#nodemethod
| elseif Set([ion_a.species, ion_b.species]) == Set([:Ti, :O]) | ||
| return 4590.7279, 0.261, 0.0 | ||
| else | ||
| # TODO: maybe throw an error here? |
There was a problem hiding this comment.
Is it better to throw an error? Is there a better way to handle this?
There was a problem hiding this comment.
If the ion pair does not belong to any of the listed pairs, the would be no Buckingham interaction.
e.g., there is no Buckingham interaction between Sr ion and Ti ion.
So I think it works pretty well for now.
There was a problem hiding this comment.
I do not think the unlisted atoms do not have Buckingham interaction
There was a problem hiding this comment.
I think the key is: In programming, we should avoid propagating the contextual information to local scopes. It seems a very unit-level function knows it will be used for computing SrTiO in the future. This is too bad. It should be well defined on its own.
There was a problem hiding this comment.
Yes, you are right. It should throw an error when receiving ion pairs other than Sr, Ti, O.
| """ | ||
| struct Lattice{D, T, L} | ||
| vectors::SMatrix{D, D, T, L} | ||
| # Q: why pbc is not used? |
There was a problem hiding this comment.
pbc is not used here, why?
There was a problem hiding this comment.
I was planning to use this method on 2D materials. A 2D lattice can be built by setting pbc = (true, true, false).
There was a problem hiding this comment.
But you did not consider it when calculating the minimum_distance.
|
@GiggleLiu The ionic radii of Ti given in the paper is 0.42 Ang. |
I see, so the ionic radii can be very different from the neutral radii, since it lost some electrons. |
Changes:
IonType.summationtoperiodic_sumbuild_ion_listtoions_on_gridbuild_vectorbuild_matrixbuild_quadratic_programtooptimize_quadraticbuild_linear_programtooptimize_linear