Skip to content

Add initialization checking in BFGS, and replace the old BFGS with a new one in CG-BFGS.#7117

Merged
19hello merged 8 commits intodeepmodeling:developfrom
19hello:fix_bfgs
Apr 1, 2026
Merged

Add initialization checking in BFGS, and replace the old BFGS with a new one in CG-BFGS.#7117
19hello merged 8 commits intodeepmodeling:developfrom
19hello:fix_bfgs

Conversation

@19hello
Copy link
Copy Markdown
Collaborator

@19hello 19hello commented Mar 23, 2026

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 March 23, 2026 07:37
Copy link
Copy Markdown

Copilot AI left a comment

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 updates the ionic relaxation flow to switch cg_bfgs into the newer/traditional BFGS implementation safely (by adding lazy initialization), and also modifies the OpenMPI stage1 build configuration.

Changes:

  • Switch cg_bfgs -> bfgs handoff to select relax_method[1] = "1" (the BFGS/bfgs_trad path).
  • Add lazy initialization (is_initialized) to BFGS so relax_step() can be called safely without a prior explicit allocate().
  • Update the OpenMPI installer to pass explicit compiler selections to ./configure.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
toolchain/scripts/stage1/install_openmpi.sh Forces compiler variables during OpenMPI configure (may affect toolchain portability).
source/source_relax/ions_move_cg.cpp Changes the cg_bfgs threshold switch to select bfgs_trad (relax_method[1]="1").
source/source_relax/bfgs.h Adds an is_initialized flag to support safe lazy initialization.
source/source_relax/bfgs.cpp Lazily calls allocate() from relax_step() if needed.

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

@mohanchen mohanchen added Bugs Bugs that only solvable with sufficient knowledge of DFT Refactor Refactor ABACUS codes GeometryRelaxation Issues related to geometry relaxation labels Mar 23, 2026
Copy link
Copy Markdown
Collaborator

@mohanchen mohanchen left a comment

Choose a reason for hiding this comment

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

LGTM

@19hello 19hello merged commit abf77eb into deepmodeling:develop Apr 1, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugs Bugs that only solvable with sufficient knowledge of DFT GeometryRelaxation Issues related to geometry relaxation Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants