Skip to content

Conversation

@Yi-FanLi
Copy link
Collaborator

This PR is a work in progress and not ready for review right not.

This PR implements the MD framework to study vibrational strong coupling based on the cavity Born-Oppenheimer approximation. It adds a new fix style fix cboamd for DeePMD-kit's LAMMPS interface.

Copilot AI review requested due to automatic review settings September 11, 2025 03:19
@Yi-FanLi Yi-FanLi marked this pull request as draft September 11, 2025 03:20
Copy link
Contributor

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 implements the MD framework to study vibrational strong coupling based on the cavity Born-Oppenheimer approximation by adding a new fix style fix cboamd for DeePMD-kit's LAMMPS interface.

  • Adds a new LAMMPS fix implementation for cavity Born-Oppenheimer molecular dynamics
  • Integrates DeepMD models for computing molecular dipoles and polarizabilities
  • Implements photon coordinate evolution using velocity Verlet integration

Reviewed Changes

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

File Description
source/lmp/plugin/deepmdplugin.cpp Registers the new fix cboamd plugin with LAMMPS
source/lmp/fix_cboamd.h Header file defining the FixCBOAMD class interface and member variables
source/lmp/fix_cboamd.cpp Implementation of the cavity Born-Oppenheimer MD fix with DeepMD integration

Comment on lines 588 to 593
fprintf(output_file, "%d %.6f %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e",
current_step, update->dt * current_step,
dipole[0], dipole[1], dipole[2],
polarizability[0], polarizability[1], polarizability[2],
polarizability[3], polarizability[4], polarizability[5],
polarizability[6], polarizability[7], polarizability[8]);
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

The output format writes 15 values but the header comment at line 160 indicates it should write Energy which is missing. The scalar energy value from compute_scalar() is not being written to the output file, creating a mismatch between the header and data columns.

Suggested change
fprintf(output_file, "%d %.6f %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e",
current_step, update->dt * current_step,
dipole[0], dipole[1], dipole[2],
polarizability[0], polarizability[1], polarizability[2],
polarizability[3], polarizability[4], polarizability[5],
polarizability[6], polarizability[7], polarizability[8]);
double energy = compute_scalar();
fprintf(output_file, "%d %.6f %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e %.6e",
current_step, update->dt * current_step,
dipole[0], dipole[1], dipole[2],
polarizability[0], polarizability[1], polarizability[2],
polarizability[3], polarizability[4], polarizability[5],
polarizability[6], polarizability[7], polarizability[8],
energy);

Copilot uses AI. Check for mistakes.
Comment on lines 297 to 301
for (int i=0; i<nphoton; i++) {
if (n == i) return pa[i];
if (n == nphoton+i) return qa[i];
if (n == 2*nphoton+i) return ea[i];
}
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

The vector size is set to 3 at line 67, but compute_vector() attempts to return values for up to 3*nphoton indices. This will cause out-of-bounds access when n >= 3 and nphoton > 1, potentially leading to crashes or undefined behavior.

Copilot uses AI. Check for mistakes.
int idx = dp * nlocal * 3 + i * 3 + di; // index in dipole_grad_deepmd: dipole component dp, atom i, coordinate di
for (int alpha = 0; alpha < nphoton; alpha++) {
// CBOA force contribution from photon alpha
f[i][di] -= ea[alpha] * lambda_photon[alpha] * lambda_vector[alpha][di] * dipole_grad_deepmd[idx] / EV_TO_HARTREE;
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

Indexing error in lambda_vector access. The code uses lambda_vector[alpha][di] but should use lambda_vector[alpha][dp] since the lambda_vector represents the coupling vector for dipole component dp, not the atomic coordinate direction di.

Suggested change
f[i][di] -= ea[alpha] * lambda_photon[alpha] * lambda_vector[alpha][di] * dipole_grad_deepmd[idx] / EV_TO_HARTREE;
f[i][di] -= ea[alpha] * lambda_photon[alpha] * lambda_vector[alpha][dp] * dipole_grad_deepmd[idx] / EV_TO_HARTREE;

Copilot uses AI. Check for mistakes.
static constexpr double EV_TO_HARTREE = 0.0367493;
static constexpr double ANGSTROM_TO_BOHR = 1.88973;
static constexpr double EV_PER_ANGSTROM_TO_HARTREE_PER_BOHR = 0.0194467;
static constexpr double PS_TO_AU = 41.3413745758e3; // 1 fs = 41.3413745758 a.u. of time
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

The comment incorrectly states '1 fs' when the constant name indicates picoseconds (PS). The comment should read '1 ps = 41.3413745758e3 a.u. of time' to match the PS_TO_AU constant name.

Suggested change
static constexpr double PS_TO_AU = 41.3413745758e3; // 1 fs = 41.3413745758 a.u. of time
static constexpr double PS_TO_AU = 41.3413745758e3; // 1 ps = 41.3413745758 a.u. of time

Copilot uses AI. Check for mistakes.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 11, 2025

Caution

Review failed

The head commit changed during the review from 031f9c0 to 2f88d06.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

FLERR,
"fix cboamd: lambda_vector must be specified when photons enabled");
}
// if (dt <= 0.0) error->all(FLERR,"fix cboamd: dt must be > 0");

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
init_deepmd_models();

// Open output file
output_file = fopen("cboamd_output.dat", "w");

Check failure

Code scanning / CodeQL

File created without restricting permissions High

A file may be created here with mode 0666, which would make it world-writable.
Comment on lines +268 to +271
// // Initialize DeepMD calculations
// convert_coordinates_to_deepmd_format();
// compute_deepmd_dipole();
// // compute_deepmd_polarizability();

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Comment on lines +273 to +275
// if (photons_enabled) {
// compute_cboa_forces();
// }

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Comment on lines +300 to +303
// Update photon coordinates if enabled
// if (photons_enabled) {
// update_photon_coordinates();
// }

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
// Compute DeepMD properties
convert_coordinates_to_deepmd_format();
compute_deepmd_dipole();
// compute_deepmd_polarizability();

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Comment on lines +528 to +531
// Extract dipole components (DeepMD returns in eV/A, convert to a.u.)
// dipole[0] = dipole_deepmd[0] * ANGSTROM_TO_BOHR;
// dipole[1] = dipole_deepmd[1] * ANGSTROM_TO_BOHR;
// dipole[2] = dipole_deepmd[2] * ANGSTROM_TO_BOHR;

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
void FixCBOAMD::write_output() {
// Write output to file
if (comm->me == 0) {
fprintf(output_file,

Check warning

Code scanning / CodeQL

Too few arguments to formatting function Medium

Format for fprintf expects 15 arguments but given 14
@codecov
Copy link

codecov bot commented Sep 11, 2025

Codecov Report

❌ Patch coverage is 1.81818% with 324 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.82%. Comparing base (40f0b1c) to head (61c7289).

Files with missing lines Patch % Lines
source/lmp/fix_cboamd.cpp 0.00% 323 Missing ⚠️
source/lmp/plugin/deepmdplugin.cpp 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #4972      +/-   ##
==========================================
- Coverage   84.27%   83.82%   -0.46%     
==========================================
  Files         705      706       +1     
  Lines       69209    69548     +339     
  Branches     3573     3646      +73     
==========================================
- Hits        58325    58297      -28     
- Misses       9743    10112     +369     
+ Partials     1141     1139       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant