Skip to content
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.

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

feat: Running CNs added to SiteRDF #1819

Merged
merged 14 commits into from
Apr 17, 2024
Merged

Conversation

Danbr4d
Copy link
Contributor

@Danbr4d Danbr4d commented Mar 14, 2024

Added into SiteRDF the functionality to calculate the running coordination number for a configuration

@Danbr4d Danbr4d added Type: Enhancement Enhancement for existing feature Scope: Modules Related to a specific Module labels Mar 14, 2024
@Danbr4d Danbr4d requested a review from trisyoungs March 14, 2024 16:35
@Danbr4d Danbr4d self-assigned this Mar 14, 2024
@Danbr4d Danbr4d marked this pull request as draft March 14, 2024 16:35
@Danbr4d Danbr4d marked this pull request as ready for review March 18, 2024 12:47
Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

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

Close! The ultimate result at the maximum of the distanceRange_ is indeed the integral of the whole histogram, and every bin up to that point is the sum of all those before it. So what you can do here is create your sumRunNHist histogram (and maybe rename to runningCNHist or similar) using the same distanceRange_ as histAB.

Then, you can simply step over the bins in each with a zip and keep track of the running bin total from histAB as you go, adding those total values at each bin into your new histogram. Note that we should do this for the raw (i.e. instantaneous, just calculated) bins of histAB so that we can form an average.

@Danbr4d Danbr4d requested a review from trisyoungs March 20, 2024 10:57
@@ -98,6 +98,35 @@ Module::ExecutionResult SiteRDFModule::process(ModuleContext &moduleContext)
}
}

auto &dataRunningCN = processingData.realise<SampledData1D>("RunningCNTest", name(), GenericItem::InRestartFileFlag);
dataRunningCN.initialise(distanceRange_.x);
Copy link
Member

Choose a reason for hiding this comment

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

Careful - distanceRange_.x is just the minimum value of the distance range to use. In fact, you don't need to initialise this class at all, since that will happen automatically on the first addition (+=) to it later on.

Suggested change
dataRunningCN.initialise(distanceRange_.x);

Comment on lines 103 to 113
std::vector<double> runningCN;

// Zip over all distanceRange and calculate running CN
double countCN{};
for (const auto &&[x, currentCN] : zip(histAB.binCentres(), histAB.data().values()))
{
runningCN[x] += currentCN;
}

// Add data into SampledData1D
dataRunningCN += runningCN;
Copy link
Member

Choose a reason for hiding this comment

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

In fact, we can do all of this summation in a std::transform...

Suggested change
std::vector<double> runningCN;
// Zip over all distanceRange and calculate running CN
double countCN{};
for (const auto &&[x, currentCN] : zip(histAB.binCentres(), histAB.data().values()))
{
runningCN[x] += currentCN;
}
// Add data into SampledData1D
dataRunningCN += runningCN;

src/modules/siteRDF/process.cpp Outdated Show resolved Hide resolved

// Normalise by A site population
normaliserInstBinValues.normaliseDivide(double(a.sites().size()));

Copy link
Member

Choose a reason for hiding this comment

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

Here we go. So we have the normalised bin data in instBinValues, and we can convert that into the running coordination numbers in-place by doing this nice std::transform. Essentially we just step over each value and calculate and return a replacement value which is the running coordination number (which we keep track of the in the sum variable):

Suggested change
auto sum = 0.0;
std::transform(instBinValues.values().begin(), instBinValues.values().end(), instBinValues.values().begin(), [&](const auto &currentBin) {sum += currentBin; return sum; } );

@trisyoungs trisyoungs changed the title Running CN added to SiteRDF feat: Running CNs added to SiteRDF Apr 8, 2024
@Danbr4d Danbr4d force-pushed the running-coordination-number branch from 69b5861 to 21f3ca2 Compare April 8, 2024 15:25
@trisyoungs trisyoungs merged commit d3c74d1 into develop Apr 17, 2024
10 checks passed
@trisyoungs trisyoungs deleted the running-coordination-number branch April 17, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Modules Related to a specific Module Type: Enhancement Enhancement for existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants