ACEsuit / Polynomials4ML.jl

Polynomials for ML: fast evaluation, batching, differentiation
MIT License
12 stars 5 forks source link

WIP: Sphericart integration #83

Closed CheukHinHoJerry closed 3 months ago

CheukHinHoJerry commented 7 months ago

This PR aims at integrating SpheriCart https://github.com/lab-cosmo/sphericart to p4ml. Todo list that I come up with but feel free to update edit. This also fixes a bug that the current real solid harmonics returns NaN when an (approximately) zero vector is given or does not evaluate.

If we want to retire the old backend entirely (though this sounds to me too ambitious):

CC @zhanglw0521 @TSGut

cortner commented 7 months ago

If we want to retire the old interface entirely

What do you mean by "the old interface"? The interface should remain the same only the backend - i.e. the code that generates the basis will be sphericart?

cortner commented 7 months ago

I think all your points are very doable and should be done. Thank you for picking this up.

CheukHinHoJerry commented 7 months ago

If we want to retire the old interface entirely

What do you mean by "the old interface"? The interface should remain the same only the backend - i.e. the code that generates the basis will be sphericart?

Sorry that was a typo - yes I mean backend.

CheukHinHoJerry commented 6 months ago

Something like this should give a correct scaling up to a sign. This comes from the Condon-Shortley Sign Convention. Is there a better way to fix this other than modifying the sign at the end of the output directly?


function generate_Flms(L::Integer; normalisation = :p4ml, T = Float64)
   Flm = OffsetMatrix(zeros(L+1, L+1), (-1, -1))
   for l = 0:L
      Flm[l, 0] = sqrt(2)
      for m = 1:l 
         Flm[l, m] = Flm[l, m-1] / sqrt((l+m) * (l+1-m))
      end
   end
   return Flm
end
cortner commented 6 months ago

Are you saying that you need to rescale the Ylm output vector by a sign after the computation, and this cannot be done at the stage of precomputing the Flms?

CheukHinHoJerry commented 6 months ago

Are you saying that you need to rescale the Ylm output vector by a sign after the computation, and this cannot be done at the stage of precomputing the Flms?

Yes exactly.

cortner commented 6 months ago

this is unfortunate. But do we really care about the sign convention?

cortner commented 6 months ago

@DexuanZhou -- what do you think? Can you also ask Huajie? (i can't find his github name right now)

hjchen1983 commented 6 months ago

@DexuanZhou -- what do you think? Can you also ask Huajie? (i can't find his github name right now)

Sorry I just saw this message. In our current simulation of Schrodinger equations, I think the sign convention is not a problem, we will be happy as long as it is a complete basis.

But I am not sure whether this could become an issue in future project. For example, when we consider some symmetry of the system and evaluate the Clebsch-Gordan coefficients?

DexuanZhou commented 6 months ago

Using it as a one-particle basis function, I agree with Huajie that the sign convention shouldn't matter much. The only potential issue arises when we compare with chemical basis functions, where they might express one-particle basis as f = a_1f_1+a_2f_2+a_3f_3. In such cases, we need to be very careful to ensure that the coefficients align properly; for instance, adjusting coefficients to -a1, a2, and a3 as needed.

DexuanZhou commented 6 months ago

My point is, if the sign convention proves difficult to reconcile, we can compensate for it by adjusting the coefficients in the combination of basis functions. This ensures that the overall wave function aligns with the chemical basis functions for comparison purposes.

cortner commented 6 months ago

That sound to me like we should try to recover the correct signs. I'll discuss with Jerry.

cortner commented 6 months ago

... on the other hand we can just fix the signs in the clebsch-gordans ...

cortner commented 3 months ago

this PR seems to have gone stale. What shall we do?

CheukHinHoJerry commented 3 months ago

I remember there is a normalization to be fixed before merging, but for this reason this can only be done on Sphercart end.

Are you saying that you need to rescale the Ylm output vector by a sign after the computation, and this cannot be done at the stage of precomputing the Flms?

cortner commented 3 months ago

So exactly what is needed at the Sphericart end?

CheukHinHoJerry commented 3 months ago

We didn't arrive at a conclusion but we decide to make a issue on that end to discuss it (which I forgot to, sorry about that). I just skim over the code again, it make sense to me just to scale the output in P4ML?

cortner commented 3 months ago

Let's discuss at our next meeting what we should do.

cortner commented 3 months ago

Please double-check whether #84 makes this PR obsolete and if you agree, then please close it.