Skip to content

refactoring: Create class for coordination number calculation #43

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
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

refactoring: Create class for coordination number calculation #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5f035a2
refactor: move charge models to new multicharge namespace and create …
Thomas3R Jun 6, 2025
7fec355
refactor: create class for coordination number calculation
Thomas3R Jun 12, 2025
b85b2c7
Update comment in dftd_ncoord.cpp
Thomas3R Jun 13, 2025
6886aa7
Move cn and dcndr allocation into ncoord and dncoord functions.
Thomas3R Jun 18, 2025
c65dc27
NCoordBase constructor initializer list and code cleanup
Thomas3R Jun 18, 2025
b2a6fc0
rename functions for consistency
Thomas3R Jun 18, 2025
fc8baed
Leave deletion of arrays to object destructor
Thomas3R Jun 18, 2025
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
73 changes: 73 additions & 0 deletions include/dftd_ncoord.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,79 @@

namespace dftd4 {


class NCoordBase
{
public:
TVector<double> cn;
TMatrix<double> dcndr;
static const double rad[];
double kcn; // Steepness of counting function
double norm_exp;
double cutoff;
// Get the coordination number
int get_ncoord( // with ghost atoms
const TMolecule&,
const TMatrix<double>&,
const double,
TVector<double>&,
TMatrix<double>&,
bool);
int get_ncoord( // without ghost atoms
const TMolecule&,
const TIVector&,
const TMatrix<double>&,
bool);
// Calculate the coordination number using the virtual counting function
int ncoord_base(
const TMolecule&,
const TIVector&,
const TMatrix<double>&);
// Calculate the derivative of the coordination number
int dncoord_base(
const TMolecule&,
const TIVector&,
const TMatrix<double>&);
// Get the DFT-D4 coordination number
Copy link
Member

Choose a reason for hiding this comment

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

The D4 CN should not be part of the base class. In the Fortran code, this is also a separate subclass (of an error function CN class).

Copy link
Contributor Author

@Thomas3R Thomas3R Jun 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think the best way would be to include the den factor in the countf:

cpp-d4/src/dftd_ncoord.cpp

Lines 460 to 461 in d61715f

den = k4 * exp(-pow((fabs(en[izp] - en[jzp]) + k5), 2) / k6);
countf = den * erf_count(kn, rr);

Then we could create a new derived type of the NCoordBase e.g. NCoordErfD4 that uses ncoord_base. This way we could delete all the "doublicate" _d4 functions and would implement consistent dimensions for dcndr(N, 3N).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just realized that changing dcndr(3N, N) to dcndr(N, 3N) is a lot more work than I thought and should be a PR for itself. Since I can not merge the _d4 functions and into the base class functions without this I will not implement this for now.

int get_ncoord_d4(
const TMolecule&,
const TMatrix<double>&,
bool);
int get_ncoord_d4(
const TMolecule&,
const TIVector&,
const TMatrix<double>&,
bool);
int ncoord_d4(
const TMolecule&,
const TIVector&,
const TMatrix<double>&);
int dncoord_d4(
const TMolecule&,
const TIVector&,
const TMatrix<double>&);

// Counting function
virtual double count_fct(double) const = 0;
// Derivative of the counting function
virtual double dcount_fct(double) const = 0;
// Constructor
NCoordBase(double optional_kcn = 7.5, double optional_norm_exp = 1.0, double optional_cutoff = 25.0);
// Virtual destructor
virtual ~NCoordBase() = default;
};

class NCoordErf : public NCoordBase {
public:
// erf() based counting function
double count_fct(double) const override;
// derivative of the erf() based counting function
double dcount_fct(double) const override;
// Constructor
NCoordErf(double optional_kcn = 7.5, double optional_norm_exp = 1.0, double optional_cutoff = 25.0)
: NCoordBase(optional_kcn, optional_norm_exp, optional_cutoff){}
};

/**
* Calculate all distance pairs and store in matrix.
*
Expand Down
16 changes: 5 additions & 11 deletions src/dftd_dispersion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,14 @@ int get_dispersion(
dist.NewMatrix(nat, nat);
calc_distances(mol, realIdx, dist);

TVector<double> cn; // D4 coordination number
TVector<double> q; // partial charges from EEQ model
TMatrix<double> dcndr; // derivative of D4-CN
TMatrix<double> dqdr; // derivative of partial charges
TVector<double> gradient; // derivative of dispersion energy
NCoordErf ncoord_erf;
multicharge::EEQModel chrg_model; // Charge model

cn.NewVector(nat);
q.NewVector(nat);
if (lgrad) {
dcndr.NewMatrix(3 * nat, nat);
dqdr.NewMatrix(3 * nat, nat);
gradient.NewVector(3 * nat);
}
Expand All @@ -122,7 +119,7 @@ int get_dispersion(
if (info != EXIT_SUCCESS) return info;

// get the D4 coordination number
info = get_ncoord_d4(mol, realIdx, dist, cutoff.cn, cn, dcndr, lgrad);
info = ncoord_erf.get_ncoord_d4(mol, realIdx, dist, lgrad);
if (info != EXIT_SUCCESS) return info;

// maximum number of reference systems
Expand All @@ -145,7 +142,7 @@ int get_dispersion(
dgwdq.NewMatrix(mref, nat);
}
info = d4.weight_references(
mol, realIdx, cn, q, refq, gwvec, dgwdcn, dgwdq, lgrad
mol, realIdx, ncoord_erf.cn, q, refq, gwvec, dgwdcn, dgwdq, lgrad
);
if (info != EXIT_SUCCESS) return info;

Expand Down Expand Up @@ -212,11 +209,10 @@ int get_dispersion(
dgwdq.NewMatrix(mref, nat);
}
info = d4.weight_references(
mol, realIdx, cn, q, refq, gwvec, dgwdcn, dgwdq, lgrad
mol, realIdx, ncoord_erf.cn, q, refq, gwvec, dgwdcn, dgwdq, lgrad
);
if (info != EXIT_SUCCESS) return info;

cn.Delete();
q.Delete();
refq.Delete();

Expand Down Expand Up @@ -253,7 +249,6 @@ int get_dispersion(
);
if (info != EXIT_SUCCESS) return info;
} else {
cn.Delete();
q.Delete();
refq.Delete();
gwvec.Delete();
Expand All @@ -266,9 +261,8 @@ int get_dispersion(
dc6dcn.DelMat();
dc6dq.DelMat();

if (lgrad) { BLAS_Add_Mat_x_Vec(gradient, dcndr, dEdcn, false, 1.0); }
if (lgrad) { BLAS_Add_Mat_x_Vec(gradient, ncoord_erf.dcndr, dEdcn, false, 1.0); }

dcndr.DelMat();
dEdcn.DelVec();
dEdq.DelVec();

Expand Down
14 changes: 5 additions & 9 deletions src/dftd_eeq.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,23 +72,19 @@ int ChargeModel::get_charges(
bool lverbose{false};
int nat = realIdx.Max() + 1;

TVector<double> cn; // EEQ cordination number
TMatrix<double> dcndr; // Derivative of EEQ-CN

cn.NewVec(nat);
if (lgrad) dcndr.NewMat(nat, 3 * nat);
dftd4::NCoordErf ncoord_erf;

// get the EEQ coordination number
info = get_ncoord_erf(mol, realIdx, dist, cutoff, cn, dcndr, lgrad);
info = ncoord_erf.get_ncoord(mol, realIdx, dist, lgrad);
if (info != EXIT_SUCCESS) return info;

// corresponds to model%solve in Fortran
info =
eeq_chrgeq(mol, realIdx, dist, charge, cn, q, dcndr, dqdr, lgrad, lverbose);
eeq_chrgeq(mol, realIdx, dist, charge, ncoord_erf.cn, q, ncoord_erf.dcndr, dqdr, lgrad, lverbose);
if (info != EXIT_SUCCESS) return info;

dcndr.DelMat();
cn.DelVec();
ncoord_erf.dcndr.DelMat();
ncoord_erf.cn.DelVec();

return EXIT_SUCCESS;
};
Expand Down
Loading