-
Notifications
You must be signed in to change notification settings - Fork 569
Implement vector fitting to replace external vectfit
package
#3493
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: develop
Are you sure you want to change the base?
Conversation
This PR is currently a draft. The code is a copy-pasting of the suggestion by chatGPT provided by Paul. |
Hello guys, I've taken the time to re-implement the code using NumPy and SciPy. The unit tests are adapted from Jingang Liang’s implementation for consistency. To validate the results, I generated WMP data from ENDF/B-VII for both Na-23 and Fe-56, and ran both implementations on these datasets. I will soon share plots comparing the absolute error in absorption cross sections between the two codes but there almost 0. I also benchmarked the compute time, though I haven’t yet post-processed those results. In any case, I believe this additional performance data isn’t critical for this PR. |
vectfit
package
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.
@azimgivron Thanks so much for going through this conversion! I made a few small updates on your branch but otherwise it looks pretty good. I do have one important question about a scaling below (wondering if this is affecting the results) below:
Hi, is this PR blocked because something is expected from me ? |
@azimgivron Any chance you can update the results you had shared above with the normalization correction? I'm curious how much of the relative error shown before may be due to that. |
Description
This pull request removes the dependency on the unmaintained external
vectfit
package (https://github.com/liangjg/vectfit) used in OpenMC's windowed multipole (WMP) infrastructure. In its place, a vector fitting is introduced based on thevectorFitting.py
module fromscikit-rf
project.The implementation will be made in several steps as suggested by Paul:
This change improves long-term maintainability and compatibility of OpenMC.
Fixes #3487
Checklist