Skip to content

Conversation

@AJPfleger
Copy link
Contributor

@AJPfleger AJPfleger commented Jan 14, 2026

  • remove extra parameter from factorial
  • add overflow boundaries and confine to defined input parameters
  • add compile time checks
  • move the tests for binomial from polynomial-tests to math-tests
  • rewrite the factorial function as a loop instead of a recursion. Since we don't do memoization.
  • derive polynomials in a recursion

@AJPfleger AJPfleger added this to the next milestone Jan 14, 2026
@github-actions github-actions bot added the Component - Core Affects the Core module label Jan 14, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2026

📊: Physics performance monitoring for 8275c92

Full contents

physmon summary

@junggjo9
Copy link
Contributor

I very strongly advert for the solution proposed in #4962

Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

just a few minor comments

I am inclined to put this in first as it seems simpler and ready compared to #4962 - what do you think @junggjo9 ? since this is ready and fixes a couple of issues. afterwards we can still iterate on the details

let me also ping @paulgessinger and @benjaminhuth on the error handling question

@junggjo9
Copy link
Contributor

Do we now have 3 PRs open on this refactoring? Really?

@paulgessinger
Copy link
Member

paulgessinger commented Jan 16, 2026

@junggjo9 I opened the last one only to show it to @andiwand. I'm happy to go with one of the others.

Can we converge this one though? I'd like to resolve the red X on the main branch if we can.

@AJPfleger
Copy link
Contributor Author

After some discussion with @andiwand , I we came to the following idea:

  • assert for unsupported parameters
  • have a compile time guard too. initially thought of static_assert but that doesn't work on the variables, so the standard solution I found is a throw inside a std::is_const_evaluated() if

@sonarqubecloud
Copy link

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

Labels

Component - Core Affects the Core module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants