Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
19hello wants to merge 2 commits into
base: develop
Choose a base branch
from
+14 −2
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a18f167
Optimize install_openmpi.sh script
Jan 16, 2026
311000c
fix cg-bfgs problem, add initialization in bfgs
Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions source/source_relax/bfgs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,16 @@ void BFGS::allocate(const int _size)
force0 = std::vector<double>(3*size, 0.0);
force = std::vector<ModuleBase::Vector3<double>>(size, ModuleBase::Vector3<double>(0.0, 0.0, 0.0));
steplength = std::vector<double>(size, 0.0);
is_initialized = true;
}


void BFGS::relax_step(const ModuleBase::matrix& _force,UnitCell& ucell)
{
if(!is_initialized)
{
allocate(ucell.nat);
}
Comment on lines +39 to +42
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

relax_step() now lazily calls allocate() when the object hasn't been initialized. There are existing unit tests for BFGS in source/source_relax/test/bfgs_test.cpp, but none cover this new path (calling relax_step() without a prior allocate()). Please add a test to assert that relax_step() succeeds and initializes internal state when allocate() was not called explicitly (this is particularly important for the cg_bfgs -> bfgs_trad switch).

Copilot uses AI. Check for mistakes.
GetPos(ucell,pos);
GetPostaud(ucell,pos_taud);
ucell.ionic_position_updated = true;
Expand Down
1 change: 1 addition & 0 deletions source/source_relax/bfgs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class BFGS
double maxstep;//every movement smaller than maxstep
double largest_grad;
int size;//number of atoms
bool is_initialized=false;

std::vector<double> steplength;//the length of atoms displacement
std::vector<std::vector<double>> H;//Hessian matrix
Expand Down
3 changes: 2 additions & 1 deletion source/source_relax/ions_move_cg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "ions_move_basic.h"
#include "source_base/global_function.h"
#include "source_base/global_variable.h"

using namespace Ions_Move_Basic;

double Ions_Move_CG::RELAX_CG_THR = -1.0; // default is 0.5
Expand Down Expand Up @@ -169,7 +170,7 @@ void Ions_Move_CG::start(UnitCell &ucell, const ModuleBase::matrix &force, const
< RELAX_CG_THR) // cg to bfgs by pengfei 13-8-8
{
Ions_Move_Basic::relax_method[0] = "bfgs";
Ions_Move_Basic::relax_method[1] = "2";
Ions_Move_Basic::relax_method[1] = "1";
}
Ions_Move_Basic::best_xxx = steplength;
}
Expand Down
7 changes: 6 additions & 1 deletion toolchain/scripts/stage1/install_openmpi.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,12 @@ case "${with_openmpi}" in
# else
# EXTRA_CONFIGURE_FLAGS=""
# fi
./configure CFLAGS="${CFLAGS}" \
./configure \
CC=gcc \
CXX=g++ \
FC=gfortran \
F77=gfortran \
Comment on lines +95 to +98
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

Hardcoding CC=gcc, CXX=g++, FC=gfortran, and F77=gfortran here overrides any compiler selection coming from toolchain.conf/toolchain.env (e.g., Intel/Clang toolchains, cross-compilers) and can make OpenMPI fail to build on systems without GNU Fortran installed. Consider respecting existing environment/toolchain variables (e.g., CC="${CC:-gcc}" etc.) or omitting these assignments and letting the toolchain drive the compiler choice, consistent with the other MPI install scripts.

Suggested change
CC=gcc \
CXX=g++ \
FC=gfortran \
F77=gfortran \
CC="${CC:-gcc}" \
CXX="${CXX:-g++}" \
FC="${FC:-gfortran}" \
F77="${F77:-gfortran}" \

Copilot uses AI. Check for mistakes.
CFLAGS="${CFLAGS}" \
--prefix=${pkg_install_dir} \
--libdir="${pkg_install_dir}/lib" \
--with-libevent=internal \
Expand Down
Loading