electronic-structure / SIRIUS

Domain specific library for electronic structure calculations
BSD 3-Clause "New" or "Revised" License
115 stars 40 forks source link

[enchancement] use GSL to solve radial equations #955

Closed toxa81 closed 5 months ago

toxa81 commented 5 months ago

Major refactoring of the radial solver. Custom RK4 integrator is removed in favour of GSL implementation. Bound state energies for Schroedinger and Dirac equations are in good agreement comparing with analytical results. Other tweaks:

toxa81 commented 5 months ago

cscs-ci run default

toxa81 commented 5 months ago

It looks alright to me. Since the code use GSL for RK method, it might be useful to give the user the choice about the method. RK4 is a bit borderline, I would use gsl_odeiv2_step_rk8pd as it has better mathematical properties but it might simply not be relevant for these calculations.

actually, the code does const gsl_odeiv2_step_type* T = gsl_odeiv2_step_rk8pd;. I was thinking to add a flexibility of controlling this, but then decided to fix it to rk8pd

mtaillefumier commented 5 months ago

It looks alright to me. Since the code use GSL for RK method, it might be useful to give the user the choice about the method. RK4 is a bit borderline, I would use gsl_odeiv2_step_rk8pd as it has better mathematical properties but it might simply not be relevant for these calculations.

actually, the code does const gsl_odeiv2_step_type* T = gsl_odeiv2_step_rk8pd;. I was thinking to add a flexibility of controlling this, but then decided to fix it to rk8pd

Perfect.

mtaillefumier commented 5 months ago

I have no objection to merge this PR. Most of my comments are cosmetics

toxa81 commented 5 months ago

I have no objection to merge this PR. Most of my comments are cosmetics

I will quickly do the optmisations of the divisions