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

Combine single_PAO_to_grid and single_PAO_to_grad #251

Closed tkoskela closed 8 months ago

tkoskela commented 8 months ago

Closes #244

I've merged the two identical subroutines into PAO_or_gradPAO_to_grid. The subroutine called in the inner loop is passed in as a procedure argument through an interface. As a side effect I had to always pass direction into both subroutines to make them conform to the same interface, in evaluate_pao it's just a dummy argument that does nothing. I've updated all calls to PAO_or_gradPAO_to_grid, evaluate_pao and pao_elem_derivative_2 and added a bit of explanation in comments.

Very open to a more descriptive names for the subroutine and the interface

Gets rid of OpenMP overheads in single_PAO_to_grad we were seeing earlier

Previously:

image

This PR:

image

tkoskela commented 8 months ago

I don't see any noticeable performance impact of passing the subroutine as an argument instead of selecting it with an if statement inside the loop nest. I like not having an if statement at the bottom of the loop nest from a code readability point of view.

tkoskela commented 8 months ago

Only minor aesthetic comments

Thanks! All done now