Move symbolic functions out of ntheory
#26050
Conversation
|
✅ Hi, I am the SymPy bot. I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
|
Benchmark results from GitHub Actions Lower numbers are good, higher numbers are bad. A ratio less than 1 Significantly changed benchmark results (PR vs master) Significantly changed benchmark results (master vs previous release) | Change | Before [a00718ba] | After [14a6e27d] | Ratio | Benchmark (Parameter) |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| - | 68.8±0.3ms | 45.2±0.4ms | 0.66 | integrate.TimeIntegrationRisch02.time_doit(10) |
| - | 69.0±0.8ms | 43.8±1ms | 0.63 | integrate.TimeIntegrationRisch02.time_doit_risch(10) |
| + | 19.0±0.4μs | 30.2±0.2μs | 1.59 | integrate.TimeIntegrationRisch03.time_doit(1) |
| - | 5.26±0.02ms | 2.85±0.04ms | 0.54 | logic.LogicSuite.time_load_file |
| - | 73.0±0.3ms | 29.0±0.2ms | 0.4 | polys.TimeGCD_GaussInt.time_op(1, 'dense') |
| - | 26.0±0.1ms | 17.1±0.08ms | 0.66 | polys.TimeGCD_GaussInt.time_op(1, 'expr') |
| - | 73.3±0.8ms | 29.5±0.3ms | 0.4 | polys.TimeGCD_GaussInt.time_op(1, 'sparse') |
| - | 252±2ms | 125±2ms | 0.5 | polys.TimeGCD_GaussInt.time_op(2, 'dense') |
| - | 256±1ms | 126±0.4ms | 0.49 | polys.TimeGCD_GaussInt.time_op(2, 'sparse') |
| - | 659±3ms | 374±2ms | 0.57 | polys.TimeGCD_GaussInt.time_op(3, 'dense') |
| - | 660±3ms | 375±1ms | 0.57 | polys.TimeGCD_GaussInt.time_op(3, 'sparse') |
| - | 495±4μs | 290±2μs | 0.58 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense') |
| - | 1.78±0.02ms | 1.06±0.01ms | 0.59 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense') |
| - | 5.81±0.02ms | 3.13±0.01ms | 0.54 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 444±3μs | 228±1μs | 0.51 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense') |
| - | 1.47±0.01ms | 687±4μs | 0.47 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense') |
| - | 4.98±0.07ms | 1.65±0.01ms | 0.33 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 380±8μs | 205±0.8μs | 0.54 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 2.47±0.02ms | 1.24±0.01ms | 0.5 | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 10.0±0.1ms | 4.38±0.03ms | 0.44 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 363±8μs | 169±0.6μs | 0.47 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense') |
| - | 2.49±0.02ms | 907±10μs | 0.36 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 9.74±0.09ms | 2.66±0.01ms | 0.27 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 1.01±0ms | 425±2μs | 0.42 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 1.77±0.03ms | 503±3μs | 0.28 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.85±0.08ms | 1.79±0.01ms | 0.31 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense') |
| - | 8.35±0.02ms | 1.50±0.01ms | 0.18 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse') |
| - | 286±2μs | 64.0±0.4μs | 0.22 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 3.44±0.02ms | 397±5μs | 0.12 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 3.95±0.03ms | 282±3μs | 0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 7.09±0.03ms | 1.26±0.01ms | 0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense') |
| - | 8.75±0.1ms | 843±8μs | 0.1 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse') |
| - | 5.06±0.04ms | 3.02±0.01ms | 0.6 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| - | 12.1±0.1ms | 6.71±0.05ms | 0.55 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 22.4±0.2ms | 9.11±0.1ms | 0.41 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.24±0.03ms | 878±10μs | 0.17 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 12.6±0.06ms | 7.10±0.04ms | 0.56 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse') |
| - | 103±0.6ms | 26.2±0.2ms | 0.25 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 167±0.9ms | 53.9±0.2ms | 0.32 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 175±0.5μs | 112±0.8μs | 0.64 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 359±2μs | 214±3μs | 0.6 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse') |
| - | 4.40±0.02ms | 858±3μs | 0.19 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 5.41±0.02ms | 383±2μs | 0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse') |
| - | 20.0±0.08ms | 2.86±0.06ms | 0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 22.9±0.2ms | 625±2μs | 0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse') |
| - | 478±2μs | 134±0.4μs | 0.28 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| - | 4.75±0.04ms | 622±5μs | 0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 5.36±0.01ms | 139±1μs | 0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| - | 13.5±0.2ms | 1.30±0.02ms | 0.1 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 14.0±0.09ms | 143±1μs | 0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| - | 132±0.5μs | 73.9±0.9μs | 0.56 | solve.TimeMatrixOperations.time_rref(3, 0) |
| - | 248±1μs | 87.3±0.3μs | 0.35 | solve.TimeMatrixOperations.time_rref(4, 0) |
| - | 24.1±0.04ms | 10.4±0.1ms | 0.43 | solve.TimeSolveLinSys189x49.time_solve_lin_sys |
| - | 28.6±0.3ms | 15.9±0.2ms | 0.56 | solve.TimeSparseSystem.time_linsolve_Aaug(20) |
| - | 55.0±0.2ms | 25.3±0.05ms | 0.46 | solve.TimeSparseSystem.time_linsolve_Aaug(30) |
| - | 28.3±0.3ms | 15.6±0.4ms | 0.55 | solve.TimeSparseSystem.time_linsolve_Ab(20) |
| - | 54.1±0.2ms | 25.1±0.2ms | 0.46 | solve.TimeSparseSystem.time_linsolve_Ab(30) |
Full benchmark results can be found as artifacts in GitHub Actions |
haru-44
force-pushed
the
ntheory_functions
branch
from
268f6f2
to
7ee47f6
Compare
haru-44
force-pushed
the
ntheory_functions
branch
from
f2de0fe
to
1a81880
Compare
| Symbolic functions in ``ntheory`` have been moved to ``functions``. | ||
| Specifically, the following functions. | ||
|
|
||
| * ``sympy.ntheory.factor_.primenu`` | ||
| * ``sympy.ntheory.factor_.primeomega`` | ||
| * ``sympy.ntheory.factor_.totient`` | ||
| * ``sympy.ntheory.residue_ntheory.mobius`` |
There was a problem hiding this comment.
The deprecation note here should be a bit clearer about what is happening from a user perspective.
All of these functions can be imported like from sympy import mobius which continues to work just fine after these changes as well as it did before. What is relevant for users/downstream is that code that does from sympy.ntheory.residue_ntheory import mobius should be changed to from sympy import mobius.
| Symbolic functions in ``ntheory`` have been moved to ``functions``. | ||
| Specifically, the following functions. | ||
|
|
||
| All of these functions can be imported like ``from sympy import mobius`` which | ||
| continues to work just fine after these changes as well as it did before. | ||
| However, if you are using ``from sympy.ntheory import mobius``, you need to | ||
| change to ``from sympy import mobius``. | ||
|
|
||
| * ``sympy.ntheory.factor_.primenu`` | ||
| * ``sympy.ntheory.factor_.primeomega`` | ||
| * ``sympy.ntheory.factor_.totient`` | ||
| * ``sympy.ntheory.residue_ntheory.mobius`` |
There was a problem hiding this comment.
The list of functions should come after "the following functions".
There was a problem hiding this comment.
Is this what you mean?
Symbolic functions in ``ntheory`` have been moved to ``functions``.
All of these functions can be imported like ``from sympy import mobius`` which
continues to work just fine after these changes as well as it did before.
However, if you are using ``from sympy.ntheory import mobius``, you need to
change to ``from sympy import mobius``. Specifically, the following functions.
* ...
There was a problem hiding this comment.
Yes, that is one possibility. Just when you say "the following functions" usually the list should follow immediately.
| ### Cleaning up symbolic version functions in ``ntheory`` | ||
|
|
||
| Symbolic functions in ``ntheory`` have been moved to ``functions``. | ||
| Specifically, the following functions. | ||
|
|
||
| All of these functions can be imported like ``from sympy import mobius`` which | ||
| continues to work just fine after these changes as well as it did before. | ||
| However, if you are using ``from sympy.ntheory import mobius``, you need to | ||
| change to ``from sympy import mobius``. Specifically, the following functions. | ||
|
|
||
| * ``sympy.ntheory.factor_.primenu`` | ||
| * ``sympy.ntheory.factor_.primeomega`` | ||
| * ``sympy.ntheory.factor_.totient`` | ||
| * ``sympy.ntheory.residue_ntheory.mobius`` |
There was a problem hiding this comment.
Okay here is how I suggest to word the whole thing:
### Relocate symbolic functions from ``ntheory`` to ``functions``
The following symbolic functions in ``ntheory`` have been moved to
``functions``:
* ``sympy.ntheory.factor_.primenu``
* ``sympy.ntheory.factor_.primeomega``
* ``sympy.ntheory.factor_.totient``
* ``sympy.ntheory.residue_ntheory.mobius``
Code that imports these functions from top-level like ``from sympy import
mobius`` will continue to work fine. However code that imports these from the
fully qualified module like ``from sympy.ntheory import mobius`` or ``from
sympy.ntheory.residue_ntheory import mobius`` will now see a deprecation
warning. The new location for these functions is in ``sympy.functions`` but the
intended way to import them is still from top-level like ``from sympy import
mobius``.
b80dcfe
to
157db04
Compare
haru-44
force-pushed
the
ntheory_functions
branch
from
41fbdaa
to
fc91f3c
Compare
haru-44
force-pushed
the
ntheory_functions
branch
from
de5a3d1
to
40f7f35
Compare
haru-44
force-pushed
the
ntheory_functions
branch
from
4b56f97
to
97a7ede
Compare
|
Can you include a summary in the OP of what happened with each individual function like what has been moved where, what has been renamed or deprecated, what has been added etc? Are all of the functions that have been moved importable at top-level in SymPy 1.12? The release note should say DEPRECATION rather than BREAKING CHANGE if something is deprecated but still works. The release note should also be clear about how downstream code should be expected to be changed as a result of the changes here. |
|
I would like someone else to review this. |
Fixed the OP. If there are any unclear points, please let me know. |
|
If possible, I would like to get this done in time for the 1.13 release(#26169). |
|
Looks good. Thanks @haru-44 ! |
|
thanks |
References to other Issues or PRs
ntheory#26034mobiusfromntheorytofunctions#26042Brief description of what is fixed or changed
The following functions have been changed (
mobiushas been fixed in other PRs).sympy.functionssympy.ntheory.factor_sympy.ntheory.factor_deprecated, but thekronecker_symbolfunction was added tontheoryafter the 1.12 release and deleted.npartitionsandpartitionexisted as functions with similar functions, they have been consolidated intopartition.top-levelcan be imported likefrom sympy import mobius. Other functions can be imported as described (e.g.,from sympy.functions import udivisor_sigma).sympy.ntheory.factor_._divisor_sigmasympy.ntheory.generate._primepisympy.partitions_._partition_rec,sympy.partitions_._partitionOther comments
Release Notes
is_carmichaelfromfunctionstontheory. The codefrom sympy import carmichael; carmichael.is_carmichael(1)orfrom sympy.functions import carmichael; carmichael.is_carmichael(1)needs to be modified tofrom sympy import is_carmichael; is_carmichael(1).find_carmichael_numbers_in_range,find_first_n_carmichaelsfromfunctionstontheory. The codefrom sympy import carmichael; carmichael.find_carmichael_numbers_in_range(1, 2)orfrom sympy.functions import carmichael; carmichael.find_carmichael_numbers_in_range(1, 2)needs to be modified tofrom sympy.ntheory.factor_ import find_carmichael_numbers_in_range; find_carmichael_numbers_in_range(1, 2).kronecker_symbolprimenu,primeomega,totient,reduce_totient,divisor_sigma,primepi,npartitions,legendre_symbol,jacobi_symbolfromntheorytofunctions. Code that imports these functions from top-level likefrom sympy import mobiuswill continue to work fine. However code that imports these from the fully qualified module likefrom sympy.ntheory import mobiusorfrom sympy.ntheory.residue_ntheory import mobiuswill now see a deprecation warning. The new location for these functions is insympy.functionsbut the intended way to import them is still from top-level likefrom sympy import mobius.udivisor_sigmafromntheorytofunctions. The codefrom sympy.ntheory.factor_ import udivisor_sigmaneeds to be modified tofrom sympy.functions import udivisor_sigma.