Skip to content

Precompute angle-independent radiation source - #16598

Merged
mcgratta merged 1 commit into
firemodels:masterfrom
whahnsr:optimize-radiation-source
Sep 27, 2026
Merged

mcgratta merged 1 commit into
firemodels:masterfrom
whahnsr:optimize-radiation-source

Conversation

@whahnsr

@whahnsr whahnsr commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Precompute the angle-independent radiation transport source once per spectral band and reuse it during the cylindrical, 2-D Cartesian, and 3-D Cartesian angular sweeps.

Previously, each angular sweep repeatedly evaluated:

KFST4_GAS + KFST4_PART + RSA_RAT*(SCAEFF+SCAEFF_G)*UIIOLD

for every cell. The new RTE_SOURCE array computes this quantity after UIIOLD is established and before entering the angular sweep.

Performance

Tested with the GNU/OpenMPI build using a shortened copy of Verification/Timing_Benchmarks/openmp_test64a.fds (T_END=1.5) and one OpenMP thread.

Three alternating original/optimized runs:

Version Average elapsed Average user CPU
Original 71.560 s 69.517 s
Optimized 68.613 s 66.513 s

This is a 4.12% elapsed-time reduction and a 1.043x speedup for the complete application run.

The previous dominant radiation source-expression line accounted for 10.73% of sampled cycles. After precomputation, the remaining source-array load accounted for approximately 6.4%.

Validation

All three original/optimized run pairs completed successfully. Extracted time-step diagnostics—including step size, pressure iterations, velocity and pressure errors, CFL values, VN values, and divergence extrema—were byte-for-byte identical for every pair.

Maximum resident memory was effectively unchanged in the measurements (approximately 378 MB).

@marcosvanella

Copy link
Copy Markdown
Contributor

@whahnsr interesting. Out of curiosity, did you employ an AI model at any point of this optimization work?

@whahnsr

whahnsr commented Sep 26, 2026 via email

Copy link
Copy Markdown
Contributor Author

@mcgratta

Copy link
Copy Markdown
Contributor

This looks good, but I am going to test the PR by running all the verification cases. This takes about an hour.

@mcgratta
mcgratta merged commit 1eaf2d5 into firemodels:master Sep 27, 2026
43 checks passed
@mcgratta

Copy link
Copy Markdown
Contributor

Thanks. The verification cases ran successfully in both debug and release mode.

@marcosvanella

Copy link
Copy Markdown
Contributor

Hello Marcos, Yes I used chatGPT to do most of the heavy lifting, it would have taken me 10x the time without it, but I understand all the stuff that was done and guided the work by keeping it on track. AI likes to go into rabbit holes from which it can not recover without help. Anyway I hope to have contributed some and made your work s little bit easier. Regards, Werner

Yes, this is in line with what we are seeing using AI. Thanks.

@mcgratta

Copy link
Copy Markdown
Contributor

FYI, there are notes on this page that explain what I did to verify that your changes did not "break" any test cases. This was a nice example of how to use AI because the suggested code changes were modest, and we can easily run these verification cases. My larger worry about AI is where someone (or something) suggests a massive change in the code that we will not be able to check and verify. It would be good to limit the PRs to one change at a time rather than a large collection of changes. If in future we need to do a git bisect, and we hit this one code change, it is easy to diagnose the problem. If, however, hundreds of lines have changed, then we ourselves will have to use an AI agent to detect the problem. Of course, that is the concern with AI---that we will become too reliant on it both to suggest changes and then to check those changes.

All this being said, what I am suggesting here is no different than what we suggest for code changes made the old fashioned way. Limit the commits to bite-sized changes to retain a nice "paper" trail.

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