-
Notifications
You must be signed in to change notification settings - Fork 2
Implement smooth interpolation of thermodynamic force #49
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
base: main
Are you sure you want to change the base?
Changes from all commits
660b81b
e8037ff
bb5e16c
a5d974c
de7a976
8a8dc95
5897b0c
aae3a90
cbb1067
570e030
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,13 +20,15 @@ namespace mrmd | |
| { | ||
| namespace analysis | ||
| { | ||
|
|
||
| data::MultiHistogram getAxialDensityProfile(const idx_t numAtoms, | ||
| const data::Atoms::pos_t& positions, | ||
| const data::Atoms::type_t& types, | ||
| const int64_t numTypes, | ||
| const real_t min, | ||
| const real_t max, | ||
| const int64_t numBins, | ||
| const real_t binWidth, | ||
| const int64_t axis) | ||
| { | ||
| MRMD_HOST_CHECK_GREATEREQUAL(max, min); | ||
|
|
@@ -37,15 +39,18 @@ data::MultiHistogram getAxialDensityProfile(const idx_t numAtoms, | |
| data::MultiHistogram histogram("density-profile", min, max, numBins, numTypes); | ||
| MultiScatterView scatter(histogram.data); | ||
|
|
||
| auto policy = Kokkos::RangePolicy<>(0, numAtoms); | ||
| auto kernel = KOKKOS_LAMBDA(const idx_t idx) | ||
| auto policy = Kokkos::MDRangePolicy<Kokkos::Rank<2>>({0, 0}, {numAtoms, numBins}); | ||
| auto kernel = KOKKOS_LAMBDA(const idx_t atomIdx, const idx_t binIdx) | ||
| { | ||
| MRMD_DEVICE_ASSERT_GREATEREQUAL(types(idx), 0); | ||
| MRMD_DEVICE_ASSERT_LESS(types(idx), numTypes); | ||
| auto bin = histogram.getBin(positions(idx, axis)); | ||
| if (bin == -1) return; | ||
| MRMD_DEVICE_ASSERT_GREATEREQUAL(types(atomIdx), 0); | ||
| MRMD_DEVICE_ASSERT_LESS(types(atomIdx), numTypes); | ||
| auto binPos = histogram.getBinPosition(binIdx); | ||
| auto atomPos = positions(atomIdx, axis); | ||
| auto access = scatter.access(); | ||
| access(bin, types(idx)) += 1_r; | ||
| if (atomPos >= binPos - binWidth / 2 && atomPos <= binPos + binWidth / 2) | ||
| { | ||
| access(binIdx, types(atomIdx)) += 1_r; | ||
| } | ||
| }; | ||
|
Comment on lines
+42
to
54
|
||
| Kokkos::parallel_for(policy, kernel); | ||
| Kokkos::Experimental::contribute(histogram.data, scatter); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,24 @@ ScalarView::HostMirror MultiHistogram::createGrid() const | |
| return grid; | ||
| } | ||
|
|
||
| ScalarView MultiHistogram::createGrid_d() const | ||
| { | ||
| MRMD_HOST_CHECK_GREATEREQUAL(numBins, 0); | ||
|
|
||
| ScalarView grid("grid", numBins); | ||
| const real_t min_ = min; | ||
| const real_t binSize_ = binSize; | ||
| auto policy = Kokkos::RangePolicy<>(0, numBins); | ||
| auto kernel = KOKKOS_LAMBDA(const idx_t idx) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use getBinPosition?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did it to avoid capturing the this pointer therefore I opted for this redundant but inline implementation. How can I use getBinPosition here without capturing the this pointer? |
||
| { | ||
| grid[idx] = min_ + (real_c(idx) + 0.5_r) * binSize_; | ||
| }; | ||
| Kokkos::parallel_for("MultiHistogram::scale", policy, kernel); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong label |
||
| Kokkos::fence(); | ||
|
|
||
| return grid; | ||
| } | ||
|
|
||
| MultiHistogram& MultiHistogram::operator+=(const MultiHistogram& rhs) | ||
| { | ||
| transform(*this, rhs, *this, bin_op::Add()); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing documentation to distinguish
requestedDensityGridSpacingfromrequestedDensityBinWidth. The ThermodynamicForce constructors use both parameters but their distinct purposes are not documented. Based on the implementation,gridSpacingappears to define the spacing between bin centers in the force grid, whilebinWidthdefines the width of bins used during density profile sampling. This distinction should be documented in comments for the constructor parameters to avoid confusion.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, could be a naming inconsistency that slipped my attention.