Skip to content

Fix empty collection checks in interpolation methods. Realy an edge c… - #323

Open
novitk wants to merge 4 commits into
amaggiulli:developfrom
novitk:frontier
Open

Fix empty collection checks in interpolation methods. Realy an edge c…#323
novitk wants to merge 4 commits into
amaggiulli:developfrom
novitk:frontier

Conversation

@novitk

@novitk novitk commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

…ondition, but allows the MixedInterp to work with just left interpolator.

QLNet

Thank you for contributing! Take a moment to review our contributing guidelines
to make the process easy and effective for everyone involved.

You must open an issue before embarking on any significant pull request, especially those that
add a new code or change existing tests, otherwise you risk spending a lot of time working
on something that might not end up being merged into the project.

IMPORTANT: By submitting a patch via a Pull Request, you agree to allow the project
owners to license your work under the terms of the BSD3 License.

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Tests
[ ] Other

Description

…ondition, but allows the MixedInterp to work with just left interpolator.
Copilot AI review requested due to automatic review settings July 8, 2026 21:35

Copilot AI 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.

Pull request overview

This PR updates MixedInterpolationImpl to safely handle the edge case where the second interpolation range (xBegin2_) is empty, preventing InvalidOperationException from calling .First() on an empty list during interpolation queries.

Changes:

  • Guard value, primitive, derivative, and secondDerivative against xBegin2_ being empty by checking xBegin2_.Count == 0 before calling xBegin2_.First().
  • Minor condition formatting adjustments (one remaining spacing inconsistency noted in review).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/QLNet/Math/Interpolations/MixedInterpolation.cs Outdated
Comment on lines 84 to 89
public override double value(double x)
{
if (x<(xBegin2_.First()))
if (xBegin2_.Count == 0 || x < (xBegin2_.First()))
return interpolation1_.value(x, true);
return interpolation2_.value(x, true);
}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 8, 2026 22:16

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines 84 to 88
public override double value(double x)
{
if (x<(xBegin2_.First()))
if (xBegin2_.Count == 0 || x < (xBegin2_.First()))
return interpolation1_.value(x, true);
return interpolation2_.value(x, true);
Konstantin Novitsky added 2 commits July 9, 2026 13:48
Updated `QL_REQUIRE` to validate against `xBegin.Count` instead of `size_`.
Added handling for cases where `size_ <= n_` in `SplitRanges` behavior,
ensuring both interpolations use the full range when necessary. Retained
original logic for cases where `size_ > n_`.
Copilot AI review requested due to automatic review settings July 9, 2026 17:49

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comment on lines 56 to 60
xBegin2_ = xBegin.GetRange(n_, xBegin.Count - n_);
yBegin2_ = yBegin.GetRange(n_, yBegin.Count - n_);

Utils.QL_REQUIRE(xBegin2_.Count < size_, () => "too large n (" + n + ") for " + size_ + "-element x sequence");
Utils.QL_REQUIRE(xBegin2_.Count < xBegin.Count, () => "too large n (" + n + ") for " + size_ + "-element x sequence");

Comment on lines +68 to +72
if (size_ <= n_)
{
interpolation1_ = factory1.interpolate(xBegin_, size_, yBegin_);
interpolation2_ = interpolation1_;
}
Comment on lines +68 to +72
if (size_ <= n_)
{
interpolation1_ = factory1.interpolate(xBegin_, size_, yBegin_);
interpolation2_ = interpolation1_;
}
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.

2 participants