Add fixed-cell i-PI socket driver with validated property responses - #7609
Add fixed-cell i-PI socket driver with validated property responses#7609Phorbol wants to merge 27 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| { | ||
| ModuleBase::TITLE("Driver", "driver_run"); | ||
|
|
||
| const std::string cal = PARAM.inp.calculation; |
There was a problem hiding this comment.
it is not a good idea to add new function here,
There was a problem hiding this comment.
Addressed in the later refactor. driver_run.cpp no longer contains socket-specific helper logic. The normal calculation dispatch remains calculation-based, and socket mode is handled inside Relax_Driver through the separate input flag socket_driver.
| return GlobalV::MY_RANK == IPI_RANK_ROOT; | ||
| } | ||
|
|
||
| void bcast_int(int& value) |
There was a problem hiding this comment.
we have 'bcast_int' function that can be used without writing a new one
There was a problem hiding this comment.
Addressed in the refactor. The old driver_ipi.cpp path was removed. The current socket driver uses Parallel_Common::bcast_int for integer broadcast. A small socket-local wrapper remains only to centralize MPI/non-MPI handling and to support string/header broadcast.
| const std::string message = bcast_string(root_message); | ||
| if (root_failed != 0) | ||
| { | ||
| throw std::runtime_error(message.empty() ? "i-PI socket I/O failed" : message); |
There was a problem hiding this comment.
Addressed at the driver level. Socket_Driver now reports fatal socket failures through ModuleBase::WARNING_QUIT. The lower-level IpiSocket class still uses exceptions internally for syscall/read/write/connect failures, which are caught by Socket_Driver and converted to WARNING_QUIT.
| CalculationModeGuard calculation_guard("scf"); | ||
|
|
||
| UnitCell ucell; | ||
| ucell.setup(PARAM.inp.latname, PARAM.inp.ntype, PARAM.inp.lmaxmax, PARAM.inp.init_vel, PARAM.inp.fixed_axes); |
There was a problem hiding this comment.
I would suggest not using PARAM (global variable)
There was a problem hiding this comment.
Addressed for the socket-specific path. Socket_Driver now receives the needed state explicitly through ESolver, UnitCell, Input_para, and ofs_running, and socket_driver.cpp no longer depends directly on the global PARAM object. Some existing ABACUS driver/relax infrastructure still uses PARAM, but the socket-specific implementation does not introduce the previous broad PARAM dependency.
Route calculation=socket through Relax_Driver, keep ESolver lifecycle in driver_run, and add source_relax socket transport tests.
341a302 to
fe2c2a3
Compare
| const bool socket_mode = (socket_esolver_inp.calculation == "socket"); | ||
| if (socket_mode) | ||
| { | ||
| socket_esolver_inp.calculation = "scf"; |
There was a problem hiding this comment.
Thanks for the PR, but here it seems not appropriate to set the calculation as 'socket' first and then change it to 'scf', it may better to design another variable other than calculation
There was a problem hiding this comment.
Addressed. The socket mode is no longer represented by calculation="socket". The input now keeps calculation="scf" and enables the independent boolean option socket_driver. The input checker rejects socket_driver with non-scf calculations.
| relax | ||
| OBJECT | ||
| relax_data.cpp | ||
| ipi_socket.cpp |
There was a problem hiding this comment.
seems better to name the file name as socket_ipi.cpp, consistent with the socket_driver.cpp
There was a problem hiding this comment.
Addressed. The files were renamed to socket_ipi.cpp/socket_ipi.h, and source_relax/CMakeLists.txt now builds socket_ipi.cpp.
There was a problem hiding this comment.
could you provide some examples in the 'interface' directory to demonstrate that the function can run successfully?
There was a problem hiding this comment.
Addressed. Added interfaces/ASE_interface/examples/socketio.py. It runs ABACUS through AbacusSocketIO, compares socket SCF energy/forces with normal FileIO SCF, and benchmarks repeated force evaluations. I also added documentation in docs/advanced/interface/ase.md.
| * relax: perform structure relaxation calculations, the relax_nmax parameter depicts the maximal number of ionic iterations | ||
| * cell-relax: perform cell relaxation calculations | ||
| * md: perform molecular dynamics simulations | ||
| * socket: run as a socket client for external drivers using the i-PI protocol |
There was a problem hiding this comment.
do you have an example to demonstrate how to use this feature?
There was a problem hiding this comment.
Addressed. The usage is now documented in docs/advanced/interface/ase.md, including AbacusSocketIO setup, ABACUS_SOCKET_ADDRESS endpoint formats, Unix socket vs TCP behavior, fixed-cell limitation, immutable INPUT/STRU settings, and atom-order handling.
|
Thanks @Phorbol for your contribution? Could you kindly reply to my questions? |
|
@mohanchen I have pushed follow-up changes and replied to the review threads. The current implementation now:
A few behavior details are worth noting:
Please let me know if any thread still needs further changes. |
|
This PR intentionally implements fixed-cell socket evaluations first. The i-PI POSDATA message carries cell data, but changing the ABACUS cell safely would require reinitializing cell-dependent state such as basis/grid/k-points/neighbors and possibly electronic-structure setup. To avoid silent incorrect results, AbacusSocketIO rejects cell changes on the ASE side, and the ABACUS socket driver also checks incoming POSDATA cells against the initial STRU cell and exits if they differ. The currently supported scope is repeated energy/force evaluation for structures with the same cell, atom count, species, pseudopotentials, basis, k-points, spin setting, and other core INPUT/STRU parameters. Changing any of these requires creating a new AbacusSocketIO instance and launching a new ABACUS client process. |
|
Thanks for your contribution, but I still have some questions. It would be easier to chat in another way, could you write an email to me? (mohanchen@pku.edu.cn) @Phorbol |
What's changed?
Add an opt-in, persistent fixed-cell i-PI socket client for repeated external energy, force, and stress evaluations. ABACUS uses
calculation scfwithsocket_driver true;Relax_Driverdispatches toSocket_Driver. Atomic positions can change, but cell, species, atom count, and electronic-structure settings must remain fixed.The client reads results directly from the completed ESolver frame, not from output files. POSDATA updates positions and runs SCF, then publishes energy, enabled derivatives, and convergence metadata together. GETFORCE consumes and clears that frame. Input validation covers finite values, atom counts, cell conditioning and inverse consistency, including ASE's inverse-cell layout. Wire integers use int32.
Energy is always computed.
cal_forceandcal_stressindependently enable derivatives; disabled fields are protocol padding, with presence declared inabacus.socket.properties.v1extras. SCF nonconvergence is reported asscf_converged=false; available results are returned and the caller chooses an acceptance policy. Stress is supported at a fixed cell; variable-cell updates, NPT, and cell optimization are excluded.The ASE
AbacusSocketIOadapter validates metadata and results, preserves atom ordering, enables requested properties, and restarts the client if a later request expands the property set. New requests clear cached results and convergence metadata; only fully validated responses are published.Usage and compatibility
Set
ABACUS_SOCKET_ADDRESS=localhost:31415for TCP orABACUS_SOCKET_ADDRESS=/tmp/ipi_abacus:UNIXfor a UNIX socket. AbacusSocketIO selects the endpoint automatically. No additional C++ socket library is required.Generic i-PI clients must explicitly enable all derivatives they consume: omitted force/stress switches are disabled. Clients ignoring extras cannot distinguish padding from a physical zero or read the convergence flag. AbacusSocketIO requires presence metadata for force/stress; legacy empty extras are accepted only for energy-only results. This is not unconditional backward compatibility with older ABACUS socket binaries.
INPUT Parameter Changes
socket_driver, restricted tocalculation scf.chg_extrap defaultselects first-order.cal_forceis not forced on; force and stress switches remain independent.docs/parameters.yaml,docs/advanced/input_files/input-main.md, and ASE documentation.Core Module Impact
Socket code is in
source/source_relax, registered in both CMake and Makefile object lists. KS runner publishes its current convergence state after SCF; LCAO output receives the current convergence argument while retaining the upstream EXX interface. These shared solver changes require CI regression coverage beyond socket tests.The fixed-cell hardening branch is integrated, including inverse-layout compatibility and frame-consistency fixes. Unrelated ELPA reports and historical design documents are excluded.
Exact Verification Performed
Local CMake build: GNU C++, MPI enabled for the repository test configuration, LCAO/ELPA/libXC disabled, installed FFTW/BLAS/ScaLAPACK and GTest. GTest requires C++17; production socket/parser sources were additionally checked under C++11.
cmake --build /tmp/abacus-pr7609-check/build --target \ MODULE_RELAX_socket_ipi_test MODULE_RELAX_socket_frame_test \ MODULE_RELAX_socket_driver_test MODULE_IO_read_item_serial -j 4 OMP_NUM_THREADS=1 ctest --test-dir /tmp/abacus-pr7609-check/build \ --output-on-failure \ -R 'MODULE_RELAX_socket_(ipi|frame|driver)_test|MODULE_IO_read_item_serial' PYTHONPATH=interfaces/ASE_interface \ /home/gengjianrui/bin/abacus-variable-cell-runtime/venv/bin/python \ -m unittest abacuslite.core.TestAbacusCalculator -k socket -v g++ -std=c++11 -fsyntax-only -Isource \ -Isource/source_base/module_container \ -I/opt/devtools/fftw/fftw-3.3.10/include \ source/source_relax/socket_frame.cpp \ source/source_relax/socket_ipi.cpp \ source/source_relax/socket_driver.cpp \ source/source_io/module_parameter/read_inp_sys.cpp python3 tools/03_code_analysis/agent_governance_check.py \ --base upstream/develop --head HEAD --format text git diff --checkResults: 4/4 CTest targets passed; 10/10 ASE socket tests passed; C++11 syntax checks passed. Expanded the actual Makefile OBJS_ABACUS list and verified socket_driver.o, socket_ipi.o, and socket_frame.o appear exactly once. Governance reports header-dependency warnings only.
Regression coverage includes two different consecutive geometries with corresponding energy/force/virial/convergence changes, duplicate GETFORCE rejection, invalid-next-frame rejection, nonorthogonal ASE cell layout, optional properties, and clearing stale ASE results after a failed response. The stale-cache regression failed before the fix and passed afterward.
Limits: driver tests use a fake solver and serial socket execution. No fresh full Intel build, multi-rank socket runtime test, or real-DFT socket-versus-independent-frame comparison was completed for this integrated revision. Full build/integration CI is pending; prior branch numerical results are not evidence for this revision.
Linked Issue
No linked issue; this introduces an optional external-driver interface.
Governance
New headers use standard-library types required by their public declarations and value members. No new default arguments or third-party dependency. Input docs and both build paths are updated. No global-dependency budget blocker was reported; no exception is requested.