-
Notifications
You must be signed in to change notification settings - Fork 6
Add find_points_tol for interpolation in RS_budgets.py #232
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
Add find_points_tol for interpolation in RS_budgets.py #232
Conversation
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.
Pull request overview
This pull request adds a find_points_tol parameter to the interpolate_all_stat_and_pstat_fields_onto_points function in RS_budgets.py to allow users to control the tolerance used when finding interpolation points in the underlying Probes class.
Changes:
- Added
find_points_tolas an optional parameter tointerpolate_all_stat_and_pstat_fields_onto_pointsfunction signature - Passed the
find_points_tolparameter to all threeProbesinstantiations within the function
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if_pass_points_to_rank0_only=True, | ||
| interpolation_output_fname="interpolated_fields.hdf5" | ||
| interpolation_output_fname="interpolated_fields.hdf5", | ||
| find_points_tol=None |
Copilot
AI
Jan 21, 2026
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.
Using None as the default value for find_points_tol will cause the parameter to be passed as None to the Probes class, overriding its default value. This could lead to runtime errors since the downstream find_points method expects a float, not None. Consider either conditionally passing the parameter only when it's not None, or using the same default value as the Probes class (e.g., np.finfo(np.double).eps * 10).
| max_pts=128, | ||
| output_fname = interpolation_output_fname | ||
| output_fname = interpolation_output_fname, | ||
| find_points_tol=find_points_tol |
Copilot
AI
Jan 21, 2026
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.
Passing find_points_tol=None will override the default value in the Probes class. If find_points_tol is None in the function signature, this should be conditionally passed (only if not None) or a proper default value should be used instead of None.
| max_pts=128, | ||
| output_fname = interpolation_output_fname | ||
| output_fname = interpolation_output_fname, | ||
| find_points_tol=find_points_tol |
Copilot
AI
Jan 21, 2026
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.
Passing find_points_tol=None will override the default value in the Probes class. If find_points_tol is None in the function signature, this should be conditionally passed (only if not None) or a proper default value should be used instead of None.
| max_pts=128, | ||
| output_fname = interpolation_output_fname | ||
| output_fname = interpolation_output_fname, | ||
| find_points_tol=find_points_tol |
Copilot
AI
Jan 21, 2026
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.
Passing find_points_tol=None will override the default value in the Probes class. If find_points_tol is None in the function signature, this should be conditionally passed (only if not None) or a proper default value should be used instead of None.
|
Nice! thanks! :D |
Adding a
find_points_toloptional input forinterpolate_all_stat_and_pstat_fields_onto_pointsin RS_budgets.py to be used in the call toProbes