Skip to content

Conversation

rikardn
Copy link
Contributor

@rikardn rikardn commented Sep 19, 2020

@isuruf-bot
Copy link

isuruf-bot commented Sep 19, 2020

Hi,

I've run clang-format and found that the code formatting is good.

Thanks for fixing the formatting.

@rikardn
Copy link
Contributor Author

rikardn commented Jul 25, 2021

I have separated the numeric calculation from the symbolic in this PR. For this particular PR it is a minor thing, but I think for other number theoretic functions that are slow and difficult to calculate numerically similar separations would come with a number of benefits:

  1. The numeric calculation could be used stand alone.
  2. The numeric caluculations could be imported and wrapped from other libraries in the future.
  3. For functions very slow to calculate like the mertens function we could set a limit on the "auto-calculation", defer the calculation and let a future doit function do the actual calculation. For example M(1000000000000) would take a long time to calculate and could be kept unevaluated until doit is called on the expression. This way expressions like M(1000000000000) * x would be super fast and if x is substituted for 0 in the future the calculation of M would never be needed.
  4. The current ntheory.cpp is a mix of pure numeric and "semi-symbolic" functions. It would be easier to find and use the functions if they would be clearly separated.

@rikardn
Copy link
Contributor Author

rikardn commented Sep 26, 2021

This is ready for review

@rikardn rikardn requested a review from a team September 26, 2021 16:49
@isuruf
Copy link
Member

isuruf commented Sep 26, 2021

What's the name that sympy use? I can't find it in sympy

@rikardn
Copy link
Contributor Author

rikardn commented Sep 26, 2021

It is not available in sympy as far as I know. It is in mathematica though (except for the root). See the links above.

@isuruf
Copy link
Member

isuruf commented Sep 26, 2021

Thanks. Can you add these links in the code comments as well?

@rikardn
Copy link
Contributor Author

rikardn commented Sep 26, 2021

Sure! Good point.

@rikardn
Copy link
Contributor Author

rikardn commented Sep 26, 2021

Sorry, did a bad merge.

@rikardn
Copy link
Contributor Author

rikardn commented Sep 26, 2021

Should be good now. Fixed bad merge and squashed commits. Also added function documentation with the references above.

@isuruf isuruf merged commit f87ec76 into symengine:master Sep 29, 2021
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