Skip to content

In-process Python bindings and faster ADI diffusion kernel - #29

Draft
Harishankar14 wants to merge 1 commit into
compiler-research:masterfrom
Harishankar14:kernel_optimizations
Draft

Harishankar14 wants to merge 1 commit into
compiler-research:masterfrom
Harishankar14:kernel_optimizations

Conversation

@Harishankar14

Copy link
Copy Markdown

Phase 1: SimParam::ComputeDerived(), in-memory results via SummaryRow; Simulate() accepts injected parameters and a results vector. Phase 2: direct writes instead of ChangeConcentrationBy, tightened Thomas loops, 8-line interleaving in all sweeps, a single parallel region.

Single-threaded, 1 simulated day: 1936 s -> 557 s (3.48x). Output byte-identical to master (1 and 9 days; grids 32, 37 and 50).

@Harishankar14

Copy link
Copy Markdown
Author
Screenshot_2026-09-26_22-56-30

Summary of the at present Original Build !!

@Harishankar14

Copy link
Copy Markdown
Author
Screenshot_2026-09-26_22-27-12

Above statistics are with respect to the changes made by the PR !!

Phase 1: SimParam::ComputeDerived(), in-memory results via SummaryRow;
Simulate() accepts injected parameters and a results vector.
Phase 2: direct writes instead of ChangeConcentrationBy, tightened Thomas
loops, 8-line interleaving in all sweeps, a single parallel region.

Single-threaded, 1 simulated day: 1936 s -> 557 s (3.48x).
Output byte-identical to master (1 and 9 days; grids 32, 37 and 50).
@minicusprime

Copy link
Copy Markdown

Damn !!
Improvement in the simulation speed is amazing.
I came across this simulation recently, maybe to study more and one of the factors was the speed !!

Took too much time, I guess this PR indeed fixes it !! (Atleast stats shows that )

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 51. Check the log or trigger a new build to see more.

Comment thread src/cart_tumor.cc
#include <cstdint>
#include <cstdlib>
#include <filesystem>
#include <fstream>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: included header filesystem is not used directly [misc-include-cleaner]

Suggested change
#include <fstream>
#include <fstream>

Comment thread src/cart_tumor.cc
#include <cstdlib>
#include <filesystem>
#include <fstream>
#include <iostream>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: included header fstream is not used directly [misc-include-cleaner]

Suggested change
#include <iostream>
#include <iostream>

Comment thread src/cart_tumor.cc
#include <iostream>
#include <memory>
#include <stdexcept>
#include <string>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: included header stdexcept is not used directly [misc-include-cleaner]

Suggested change
#include <string>
#include <string>

Comment thread src/cart_tumor.cc
// simulate total_minutes_to_simulate minutes including the last minute
const auto t_startup_end = std::chrono::steady_clock::now();
const double startup_ms =
std::chrono::duration<double, std::milli>(t_startup_end - t_startup_begin)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: no header providing "std::milli" is directly included [misc-include-cleaner]

src/cart_tumor.cc:46:

- #include <stdexcept>
+ #include <ratio>
+ #include <stdexcept>

Comment thread src/cart_tumor.h
/// Function declaration for the main simulation
int Simulate(int argc, const char** argv);
int Simulate(int argc, const char** argv, SimParam* injected_params = nullptr,
std::vector<SummaryRow>* results = nullptr);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: no header providing "std::vector" is directly included [misc-include-cleaner]

src/cart_tumor.h:23:

- namespace bdm {
+ #include <vector>
+ namespace bdm {

inline void ForwardBlock(real_t* __restrict c, size_t base, int jump, int n,
const real_t* __restrict d, real_t k, real_t lo,
real_t hi) {
real_t prev[W];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]

  real_t prev[W];
  ^

real_t hi) {
real_t prev[W];
for (int b = 0; b < W; ++b) {
const real_t old = c[base + b];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]

    const real_t old = c[base + b];
                       ^

real_t prev[W];
for (int b = 0; b < W; ++b) {
const real_t old = c[base + b];
const real_t v = old / d[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: do not use pointer arithmetic [cppcoreguidelines-pro-bounds-pointer-arithmetic]

    const real_t v = old / d[0];
                           ^

for (int b = 0; b < W; ++b) {
const real_t old = c[base + b];
const real_t v = old / d[0];
prev[b] = std::clamp(old + (v - old), lo, hi);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]

    prev[b] = std::clamp(old + (v - old), lo, hi);
    ^

const real_t old = c[base + b];
const real_t v = old / d[0];
prev[b] = std::clamp(old + (v - old), lo, hi);
c[base + b] = prev[b];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]

    c[base + b] = prev[b];
                  ^

@Harishankar14
Harishankar14 marked this pull request as draft October 3, 2026 11:03
@Harishankar14

Copy link
Copy Markdown
Author

Adding this PR after merging #24 into the main stream !!

@salva24

salva24 commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Hi, could you please run the following these two simulations:
2 doses:
{
"seed": 42,
"output_performance_statistics": true,
"total_minutes_to_simulate": 43200,
"initial_tumor_radius": 150.0,
"treatment": {
"0": 3957,
"8": 3957
}
}
No doses
{
"seed": 42,
"output_performance_statistics": true,
"total_minutes_to_simulate": 43200,
"initial_tumor_radius": 150.0,
"treatment": {
"0": 0
}
}
Please add screenshots with the graphs you get from running the notebook and the number of threads you used (use as many as possible).
Thank you!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants