Refactor source_psi, remove some unnecessary dependency - #7675
Refactor source_psi, remove some unnecessary dependency#7675mohanchen wants to merge 19 commits into
Conversation
There was a problem hiding this comment.
Second comment: a design suggestion on the new prepare_params() / initialize() split. Not a correctness problem, but since it is an interface shape this PR newly introduces, it is much cheaper to settle now than after callers accumulate.
(This is dyzheng play😋)
Critsium-xy
left a comment
There was a problem hiding this comment.
Three minor cleanup points, none blocking.
| */ | ||
| template <typename T> | ||
| class psi_initializer | ||
| class psi_base |
There was a problem hiding this comment.
Naming: psi_base is a weaker name than psi_initializer, and it collides conceptually with psi::Psi.
psi_initializer named the responsibility; psi_base names a position in the class hierarchy. The responsibility has not changed -- the comment right above this line still says "This class is used to allocate memory and give initial guess for psi", and the usage sequence documented there is still new->initialize->tabulate->init_psig. So the new name carries strictly less information than the old one.
The more concrete problem is that the file now sits next to source_psi/psi.h as source_psi/psi_base.h, which reads as "base class of psi::Psi". No such relationship exists, and a reader has to open the file to find that out.
If the goal is just to shorten the psi_initializer_* prefix -- and psi_init_atomic etc. are a genuine improvement -- then psi_init_base (or psi_init) keeps that gain while still describing the role, and matches the new CMake target name psi_init introduced in this PR. Worth settling before the name spreads: it is a sed today, and a much wider diff once other modules start referring to it.
Refactor source_psi, remove some unnecessary dependency