Skip to content

refactor: extract MD statistics as pure functions#7411

Open
lijianing-sudo wants to merge 1 commit into
deepmodeling:developfrom
Audrey-777:refactor/md-pure-statistics
Open

refactor: extract MD statistics as pure functions#7411
lijianing-sudo wants to merge 1 commit into
deepmodeling:developfrom
Audrey-777:refactor/md-pure-statistics

Conversation

@lijianing-sudo
Copy link
Copy Markdown

Reminder

  • Have you linked an issue with this pull request?
  • Have you added adequate unit tests and/or case tests for your pull request?
  • Have you noticed possible changes of behavior below or in the linked issue?
  • Have you explained the changes of codes in core modules of ESolver, HSolver, ElecState, Hamilt, Operator or Psi? (ignore if not applicable)

Linked Issue

Fix #... (refactoring #7: extract MD temperature, kinetic energy, and stress statistics as pure functions)

Unit Tests and/or Case Tests for my changes

No new tests are added — the behavioral surface is unchanged. All existing MODULE_MD tests pass because current_temp() and compute_stress() are preserved as thin wrappers that delegate to the new pure functions internally. The floating-point computation paths are identical.

What's changed?

Extract temperature, kinetic energy, and stress statistics from the MD_func namespace into pure functions that return explicit structs, instead of writing results through mutable reference parameters.

New file: source/source_md/md_statistics.h

  • MDKineticState — holds kinetic and temperature
  • MDStressState — holds t_vector (ionic kinetic tensor) and stress (total stress)
  • FIREProjection — reserved for future FIRE refactoring (not used yet)

source/source_md/md_func.h

  • Add #include "md_statistics.h"
  • Declare calc_kinetic_state() and calc_stress_state() — pure, no side effects

source/source_md/md_func.cpp

  • Implement calc_kinetic_state() and calc_stress_state()
  • current_temp() becomes a wrapper: calls calc_kinetic_state(), writes back to ref params
  • compute_stress() becomes a wrapper: calls calc_stress_state(), assigns stress
  • kinetic_energy() and temp_vector() are unchanged

source/source_md/nhchain.cpp — 4 call sites updated in first_half() and second_half():

// Before
t_current = MD_func::current_temp(kinetic, ucell.nat, frozen_freedom_, allmass, vel);
MD_func::compute_stress(ucell, vel, allmass, cal_stress, virial, stress);

// After
MDKineticState kstate = MD_func::calc_kinetic_state(ucell.nat, frozen_freedom_, allmass, vel);
kinetic   = kstate.kinetic;
t_current = kstate.temperature;
if (cal_stress) {
    MDStressState sstate = MD_func::calc_stress_state(ucell, vel, allmass, virial);
    stress = sstate.stress;
}

source/source_md/msst.cpp — 4 call sites updated in setup() and second_half().

Backward compatibility: md_base.cpp, verlet.cpp, run_md.cpp, and md_func_test.cpp still call the old current_temp()/compute_stress() interfaces, which continue to work unchanged.

Any changes of core modules? (ignore if not applicable)

No. All changes are confined to source/source_md/.

- Add md_statistics.h with MDKineticState, MDStressState, FIREProjection
- Add calc_kinetic_state() and calc_stress_state() pure functions
- Old interfaces (current_temp, compute_stress) kept as wrappers
- Update nhchain.cpp and msst.cpp call sites

Co-Authored-By: Person B <person.b@example.com>
@mohanchen
Copy link
Copy Markdown
Collaborator

This refactoring idea is quite interesting. Feel free to move forward with your implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants