OrderN / CONQUEST-release

Full public release of large scale and linear scaling DFT code CONQUEST
http://www.order-n.org/
MIT License
96 stars 25 forks source link

Non-integer for atomic number #52

Closed tsuyoshi38 closed 4 years ago

tsuyoshi38 commented 4 years ago

We sometimes want to use an artificial atom whose atomic number is non-integer. The present version of MakeIonFiles (in tools/BasisGeneration) cannot treat such atoms.

tsuyoshi38 commented 4 years ago

Before the release of the code, I once worked with this problem and revised the code. At that time, I introduced a new variable integer, allocatable, dimension(:) :: atomicnum in dimens_local_module.f90 and changed the definition of pseudo(ispecies)%z from integer to real. (this is real in ONCVPSP, I think.)

But, since we also have pseudo(ispecies)%zval which is calculated as a sum of occupation of each valence orbital, (in read_module.f90 ! Read n, l, filling for valence zval = zero do i_shell = 1, nv read(a,*) en,ell,fill zval = zval + fill a = get_hamann_line(lun) end do ) we may be able to use this value as the non-integer atomic number, if we assume that we always use neutral atoms in the generation of pseudopotentials.

My only concern is .. we may use ionic states for the generation of pseudopotentials. In this case, I am afraid that pseudo(:)zval may not be the atomic number. (I am not sure about it. I have not checked whether the above "fill" is filling for the configuration in the atomic calculations or not.)

Any suggestions or comments ?

tsuyoshi38 commented 4 years ago

If we can use pseudo(:)%zval as atomic number, we only need to change two lines to treat this non-integer atomic numbers. in read_module.f90 : pseudo(i_species)%z = int(z) -> pseudo(i_species)%z = nint(z) in mesh_module.f90 : mesh_z = real(pseudo(species)%z, double) -> mesh_z = pseudo(species)%zval

But, it is also easy to add "integer atominc_num".
More serious question might be whether the present PAO generation code (and ONCVPSP?) can treat ionic configurations in the generation of pseudopotentials or not.

davidbowler commented 4 years ago

The variable atomicnum is still in dimens_local_module.f90 so can be used. I think that we should not use zval for the atomic number: it is supposed to be the number of electrons, and so ionic configurations would require it to be different to z.

The release notes for ONCVPSP say that Hamann strongly discourages ionic configurations for pseudopotentials in solids, so I don't know how reliable an ionic pseudopotential would be. I've not tested ionic configurations in the PAO generation, but they should work.

The only place where z is required to be integer is when we use it as the index in selecting the element name (the array pte) so we could just change these references, and have a real z if you prefer.

davidbowler commented 4 years ago

There is also an interesting question when interpolating potentials; the local potential is interpolated so that it matches -Z/r in schro_module.f90:

https://github.com/OrderN/CONQUEST-release/blob/04505235e7dbc63c52531be3d58de24e8580a927/tools/BasisGeneration/schro_module.f90#L535-L536

I think that this should use z not zval - do you agree?

tsuyoshi38 commented 4 years ago

Thank you for your comment.
Yes, I think local potential at large r should be -z/r (if z is a real atomic number), but it should be -zval/r with the present definition of z. (z: integer at present.)

  1. I am not saying that we should use ionic configuration to generate pseudopotentials, but someone may want to try it. (I don't know why it is not recommended to use ionic configuration by ONCVPSP. )
    Can we ignore the case considering ionic configuration ? Maybe no...

  2. If we should be able to choose this option (ionic configuration), I prefer to change z as a real number. (and will not use atomicnum. I think I prepared it only for selecting the element name.) Is it okay ?

How about zval in find_eigenstate_and_energy_vkb ? This should be also z instead of zval?

One more question. Are there any other parts where zval should be used instead of z ?

davidbowler commented 4 years ago

I am happy for z to become a real number, with a nint call when it's used as the index for pte.

I don't think that zval in find_eigenstate_and_energy_vkb is critical - it's just an estimate of a lower bound for the eigenvalue. I couldn't see any other areas where the change needs to be made.

We use ONCVPSP pseudopotentials in the distribution, but there is no reason why we shouldn't use other potentials in future, so I think that we should be able to work with ionic configurations.

tsuyoshi38 commented 4 years ago

I have made a branch "f-non-integer-atomic-number_GenBasis" from "develop", and changed the code in this branch. I have just confirmed that it works for H_0.75 or H_1.25.

But... how should I link the branch to this issue ? and .. how can I issue a pull request ? (though I should tidy up the code a little more) Is it okay to issue a pull request from the branch ? Then, will it be the pull request to "develop" branch ?

tsuyoshi38 commented 4 years ago

Yes, I think local potential at large r should be -z/r (if z is a real atomic number), but it should be -zval/r with the present definition of z. (z: integer at present.)

I have noticed that my comment above is wrong. In the discussion, we probably assumed H1.25 or H0.75. But, we also use the subroutine interpolate for other elements, of course. Thus, this should be -(z-z_core)/r.
(Here, z_core is the number of core electrons. Do we have a variable showing this number ?)

I might have introduced a serious bug... Scary.

tsuyoshi38 commented 4 years ago

When I checked my version, I have found that *CQ.ion for minimal basis set, the final PAO is considered as a polarized orbital. For N, 1 2 # Lmax for basis, no of orbitals 0 2 1 0 2.000000 #orbital l, n, z, is_polarized, population 1 2 1 1 3.000000 #orbital l, n, z, is_polarized, population

I don't think this is a serious problem, but I have found that the problem also exists with the master version.

davidbowler commented 4 years ago

Resolved by #53