Skip to content

fix bfgs bug#7483

Open
19hello wants to merge 3 commits into
deepmodeling:developfrom
19hello:fix_bfgs_bug
Open

fix bfgs bug#7483
19hello wants to merge 3 commits into
deepmodeling:developfrom
19hello:fix_bfgs_bug

Conversation

@19hello

@19hello 19hello commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

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 #...

Unit Tests and/or Case Tests for my changes

  • A unit test is added for each new feature or bug fix.

What's changed?

  • Example: My changes might affect the performance of the application under certain conditions, and I have tested the impact on various scenarios...

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

  • Example: I have added a new virtual function in the esolver base class in order to ...

Copilot AI review requested due to automatic review settings June 18, 2026 06:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an early-return condition in the BFGS-based ionic relaxation update step by checking the maximum absolute displacement component, avoiding incorrect termination when all displacement components are negative.

Changes:

  • Replace max_element(dpos) threshold check with a max_abs_dpos computation using std::abs.
  • Prevent erroneous early return in Ions_Move_BFGS2::Update() when displacements are negative but non-negligible.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 241 to +247
double threshold=1e-7;
if(*max_element(dpos.begin(), dpos.end()) < threshold)
double max_abs_dpos = 0.0;
for (const double value : dpos)
{
max_abs_dpos = std::max(max_abs_dpos, std::abs(value));
}
if (max_abs_dpos < threshold)
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.

2 participants