Skip to content

[Rafactor] Remove parameter.h in rest files in source_basis#7373

Open
Critsium-xy wants to merge 7 commits into
deepmodeling:developfrom
Critsium-xy:decouple_orb_read_param
Open

[Rafactor] Remove parameter.h in rest files in source_basis#7373
Critsium-xy wants to merge 7 commits into
deepmodeling:developfrom
Critsium-xy:decouple_orb_read_param

Conversation

@Critsium-xy
Copy link
Copy Markdown
Collaborator

@Critsium-xy Critsium-xy commented May 22, 2026

Both reads were removed without changing any function signature, because in each case the value was already recoverable from information the function already had.

  1. build_alpha — guard via the existing ndesc argument
void build_alpha(int ndesc = 0, std::string* file_desc0 = nullptr)
{
    if (PARAM.globalv.deepks_setorb) { /* build alpha_ from ndesc descriptors */ }

The sole caller passes deepks_setorb as the ndesc argument:

build_alpha(PARAM.globalv.deepks_setorb, &ucell.descriptor_file);

So the flag's value already enters the function through ndesc (bool → int). Re-reading the global inside the body is redundant. The guard becomes if (ndesc > 0), which is equivalent for the caller and is also more self-consistent: build the descriptor collection only when there are descriptors to build it from. This is the "value already lives in an existing parameter" case — zero cost to fix.

  1. to_LCAO_Orbitals — guard via the object's own state

if (PARAM.globalv.deepks_setorb) { /* fill ORB.Alpha from alpha_ */ }

Here the relevant invariant is alpha_'s lifecycle: the member is only constructed in build_alpha (i.e. when DeePKS is enabled) and is otherwise null. Therefore:

alpha_ != nullptr ⟺ deepks_setorb == true

The rest of the function (and all of tabulate) already branches on if (alpha_). Replacing the global read with if (alpha_) removes the dependency, matches the surrounding style, and is safer — it null-checks the very pointer it is about to dereference. This is the "value is equivalent to existing object state" case — also zero cost.

Contrast — orbital_dir had to become a parameter

return PARAM.inp.orbital_dir + file; // path prefix

orbital_dir is genuine external input (a user-specified directory). Nothing in the function's existing arguments or state can reconstruct it, so it cannot be eliminated — only pushed up the call chain. It was added as a build_orb parameter and supplied by the two callers, which already hold PARAM. This is why only one of the three PARAM uses required a signature change.

Critsium-xy and others added 7 commits May 19, 2026 16:31
Decouple module_ao and module_nao from source_io/parameter.h:

- ORB_atomic_lm / ORB_nonlocal_lm: replace PARAM.globalv.global_out_dir
  with ModuleBase::get_quit_out_dir() (new getter mirroring the existing
  set_quit_out_dir injection point).
- two_center_bundle: thread orbital_dir as a build_orb parameter; replace
  the two deepks_setorb guards with ndesc>0 / alpha_ non-null checks that
  are equivalent under the build_alpha invariant.

source_basis is now free of parameter.h.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Critsium-xy Critsium-xy marked this pull request as ready for review May 22, 2026 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant