Move incore/OOC decision logic for DPD sorting into a separate function

[TiborGY]

Description

DPD::buf4_sort(...) has some problems, the main scope of the function is quite polluted cluttered and it is a behemoth of a function.
This PR attempts to improve that by moving the incore/out-of-core decision logic into a separate function and file. DPD::buf4_sort_axpy(...) had the same code duplicated.

The new function uses const wherever possible, its integers are now int64_t (with the exception of irrep numbers - having >2 billion irreps seems unlikely) and the incore variable is now a bool.

Todos

• DPD::buf4_sort(...) and DPD::buf4_sort_axpy(...) are slightly easier to read and debug
• Code duplication between DPD::buf4_sort(...) and DPD::buf4_sort_axpy(...) is reduced
• Possible reduction in int overflow risk via int64_t

Checklist

• CI tests are passing

Status

• Ready for review
• Ready for merge

[lothian]

Can you elaborate on what you mean by "the main scope of the function is quite polluted"?

[TiborGY]

Certainly. Polluted might have been too harsh, cluttered may be a more appropriate word for it.

All variables are currently declared at the beginning of the function. Depending on the type of sort requested, some of them may never be initialized/used, but because they are declared at the top they are always visible and mutable inside the switch cases, loops, etc.

This makes debugging more challenging than it has to be, as it is not possible to tell at a glance which of the variables with suspicious (negative or power-of-two) values are just uninitialized, as seen in the stack trace in #2261 (comment)

In general variables should enter scope when they are needed and go out of scope when they are no longer required, and be const if they are never modified.

[lothian]

Certainly. Polluted might have been too harsh, cluttered may be a more appropriate word for it.

All variables are currently declared at the beginning of the function. Depending on the type of sort requested, some of them may never be initialized/used, but because they are declared at the top they are always visible and mutable inside the switch cases, loops, etc.

This makes debugging more challenging than it has to be, as it is not possible to tell at a glance which of the variables with suspicious (negative or power-of-two) values are just uninitialized, as seen in the stack trace in #2261 (comment)

In general variables should enter scope when they are needed and go out of scope when they are no longer required, and be const if they are never modified.

All fair criticisms, and that clears it up. The code was originally pure C and later modified to fit (nominally) within a C++ framework, hence the structure of the variable declarations.