List more math function in API docs #7211

brandonhorsley · 2024-03-22T12:46:53Z

Description

This commit is a solution to the problems presented in PyMC github issue #7130. The first part of the issue was that certain functions like inverse trigonometric functions didn't seem to work, however on a clean install for this commit and pull request, that issue disappeared and the functions work as intended. These functions were missing among others from the website documentation which is the second part of the github issue and is what this particular git commit concerns. Note I have included tround although other codes reflect this as being deprecated but still able to be used. I have also rearranged the ordering to better reflect similar functions being bunched together.

Related Issue

Closes ENH: Expansion of pymc.math to include more numpy/pytensor functions #7130
[] Related to #

Checklist

Checked that the pre-commit linting/style checks pass
Included tests that prove the fix is effective or that the new feature works
Added necessary documentation (docstrings and/or example notebooks)
If you are a pro: each commit corresponds to a relevant logical change

Type of change

📚 Documentation preview 📚: https://pymc--7211.org.readthedocs.build/en/7211/

…ithub issue pymc-devs#7130. The first part of the issue was that certain functions like inverse trigonometric functions didn't seem to work, however on a clean install for this commit and pull request, that issue disappeared and the functions work as intended. These functions were missing among others from the website documentation which is the second part of the github issue and is what this particular git commit concerns. Note I have included tround although other codes reflect this as being deprecated but still able to be used. I have also rearranged the ordering to better reflect similar functions being bunched together.

welcome · 2024-03-22T12:46:56Z

]
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

brandonhorsley · 2024-03-22T13:33:37Z

Doc build worked as expected which is good news, does read a bit long so maybe further polish could be doing further category subdivisions like 'Trig functions', 'tensor functions'...etc but this would probably warrant discussion on what good subcategories could be

OriolAbril

Everything looks good, I agree with sorting them into multiple topic related subsections but can be done in a follow up PR

jessegrabowski · 2024-03-23T14:25:59Z

Do all of these functions need to be exposed? I don't think the _numpy functions need to be there, but also the linear algebra routines.

OriolAbril · 2024-03-23T15:05:58Z

Do all of these functions need to be exposed? I don't think the _numpy functions need to be there, but also the linear algebra routines.

If we expect them to be used they should be somewhere. I am under the impression it is the case, but if it were preferred to use them from pytensor then they can be removed from here and listed on pytensor only.

jessegrabowski · 2024-03-23T15:14:01Z

My only strong objection is to the numpy ones, because we shouldn't be even suggesting that users use numpy functions on pytensor objects. But I guess this PR is just updating the docs to reflect what functions are already there, so maybe this concern should go in a different thread.

OriolAbril · 2024-03-23T15:42:59Z

There is the underlying issue that it isn't really clear what is PyMC's public API, even to team members. But I think this is a good place to discuss that for these functions. Any functions we don't think are public should NOT go into the docs

brandonhorsley · 2024-03-23T17:56:07Z

Yeah I mean with the pull request I have put forward I just covered all the functions presented in the math.py file just out of sheer completeness since I didn't want to make that executive decision without consultation. I believe I also included 'tround' in the math.rst file for this pull request which is listed as deprecated in math.py but does still work so that one would be another one to possibly put onto the cutting room floor since all it does is raise a deprecation warning and call 'round' anyway

ricardoV94 · 2024-03-24T08:14:29Z

Let's just show the PyTensor functions and not list the numpy ones. Further refining can be done later.

brandonhorsley · 2024-03-24T10:36:32Z

Is that something to do on my end? If so is it worth omitting the deprecated 'tround' from the list as well or just the numpy ones?

ricardoV94 · 2024-03-24T10:42:44Z

You can actually remove the deprecated tround and yes omit the numpy ones

ricardoV94 · 2024-03-25T10:41:31Z

I actually meant removing the tround code as well

brandonhorsley · 2024-03-25T10:42:40Z

Ahh I'll do that now, remove the numpy code from math.py too?

ricardoV94 · 2024-03-25T10:44:00Z

I don't know if the numpy code is being used.

Regarding tround, you can remove that code. There's also a round wrapper that can be removed and the module changed to provide access to pytensor.tensor.round directly

ricardoV94 · 2024-03-25T10:46:03Z

If you want (totally optional), you can deprecate the _numpy functions for now with a FutureWarning. I see that one of them is being used in a test (that is not just testing the function!), but we can just define it there then, no need to be part of our public modules

Removed tround, replaced round with pytensor.tensor import, and added FutureWarning to log1mexp_numpy and logdiffexp_numpy.

pymc/math.py

Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>

Searched PyMC codebase for occurrences of _numpy functions and added explicit warnings. Most of these were for log1mexp_numpy which had quite a few involvements in test codes. Also note that on test_math.py the _numpy functions are explicitly imported and so when deprecated those imports will have to be removed too.

tests/distributions/test_continuous.py

tests/test_math.py

This commit undoes the changes to test_math.py by removing the FutureWarning warnings for the _numpy functions. Note that when these tests are removed the _numpy functions are imported explicitly in this file. Replaced the log1mexp_numpy use in test_continuous.py by just substituting in the maths from the function to avoid calling the function.

ricardoV94 · 2024-03-25T15:03:11Z

@brandonhorsley I pushed a commit that refactors a bit the test code to be more readable. Otherwise I think all is good. Tests are running now

codecov · 2024-03-25T15:23:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.89%. Comparing base (6252d2e) to head (43ec2ed).
Report is 30 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7211      +/-   ##
==========================================
- Coverage   92.23%   91.89%   -0.35%     
==========================================
  Files         100      100              
  Lines       16889    16884       -5     
==========================================
- Hits        15578    15515      -63     
- Misses       1311     1369      +58

Files	Coverage Δ
pymc/math.py	`77.71% <100.00%> (+1.82%)`	⬆️

... and 28 files with indirect coverage changes

welcome · 2024-03-27T09:30:59Z

]
Congrats on merging your first pull request! 🎉 We here at PyMC are proud of you! 💖 Thank you so much for your contribution 🎁

ricardoV94 · 2024-03-27T09:31:46Z

Thanks @brandonhorsley

brandonhorsley · 2024-03-27T09:33:25Z

Thanks for your help as well @ricardoV94 et al., you've been a big help for getting this first pull request under my belt!

ricardoV94 · 2024-03-27T09:37:13Z

You're welcome. Looking for your second if that happens!

Also removes deprecated functions and deprecates numpy helpers Co-authored-by: Ricardo Vieira <ricardo.vieira1994@gmail.com>

brandonhorsley marked this pull request as ready for review March 22, 2024 13:33

ricardoV94 changed the title ~~This commit is a partial solution to the problems presented in PyMC g…~~ List more math function in API docs Mar 22, 2024

ricardoV94 added the docs label Mar 22, 2024

ricardoV94 requested review from jessegrabowski and OriolAbril March 22, 2024 19:35

OriolAbril approved these changes Mar 23, 2024

View reviewed changes

Removed numpy functions and tround from math.rst

a35a40e

ricardoV94 approved these changes Mar 25, 2024

View reviewed changes

Edits to math.py.

a971005

Removed tround, replaced round with pytensor.tensor import, and added FutureWarning to log1mexp_numpy and logdiffexp_numpy.

jessegrabowski approved these changes Mar 25, 2024

View reviewed changes

ricardoV94 reviewed Mar 25, 2024

View reviewed changes

pymc/math.py Outdated Show resolved Hide resolved

ricardoV94 requested changes Mar 25, 2024

View reviewed changes

pymc/math.py Outdated Show resolved Hide resolved

brandonhorsley and others added 3 commits March 25, 2024 11:20

Update pymc/math.py

0f43005

Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>

Update pymc/math.py

efa4543

Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>

ricardoV94 reviewed Mar 25, 2024

View reviewed changes

tests/distributions/test_continuous.py Outdated Show resolved Hide resolved

tests/test_math.py Outdated Show resolved Hide resolved

brandonhorsley and others added 3 commits March 25, 2024 13:34

Removed a FutureWarning I forgot to remove

b14e000

refactor helper function

7f3eb36

Tweak tests

43ec2ed

ricardoV94 approved these changes Mar 27, 2024

View reviewed changes

ricardoV94 merged commit 81d31c8 into pymc-devs:main Mar 27, 2024
23 checks passed

This was referenced Apr 8, 2024

Bug fixes for statespace pymc-devs/pymc-experimental#326

Merged

CI fails after pymc-devs/pymc#7211 and pymc-devs/pymc#7238 pymc-devs/pymc-experimental#328

Closed

Handle new pymc and pytensor releases pymc-devs/pymc-experimental#329

Merged

mkusnetsov pushed a commit to mkusnetsov/pymc that referenced this pull request Oct 26, 2024

List more math function in API docs (pymc-devs#7211)

16e6d69

Also removes deprecated functions and deprecates numpy helpers Co-authored-by: Ricardo Vieira <ricardo.vieira1994@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

List more math function in API docs #7211

List more math function in API docs #7211

brandonhorsley commented Mar 22, 2024 •

edited

Loading

welcome bot commented Mar 22, 2024

brandonhorsley commented Mar 22, 2024

OriolAbril left a comment

jessegrabowski commented Mar 23, 2024

OriolAbril commented Mar 23, 2024

jessegrabowski commented Mar 23, 2024

OriolAbril commented Mar 23, 2024

brandonhorsley commented Mar 23, 2024

ricardoV94 commented Mar 24, 2024

brandonhorsley commented Mar 24, 2024

ricardoV94 commented Mar 24, 2024

ricardoV94 commented Mar 25, 2024 •

edited

Loading

brandonhorsley commented Mar 25, 2024

ricardoV94 commented Mar 25, 2024 •

edited

Loading

ricardoV94 commented Mar 25, 2024 •

edited

Loading

ricardoV94 commented Mar 25, 2024

codecov bot commented Mar 25, 2024 •

edited

Loading

welcome bot commented Mar 27, 2024

ricardoV94 commented Mar 27, 2024

brandonhorsley commented Mar 27, 2024

ricardoV94 commented Mar 27, 2024

List more math function in API docs #7211

List more math function in API docs #7211

Conversation

brandonhorsley commented Mar 22, 2024 • edited Loading

Description

Related Issue

Checklist

Type of change

welcome bot commented Mar 22, 2024

brandonhorsley commented Mar 22, 2024

OriolAbril left a comment

Choose a reason for hiding this comment

jessegrabowski commented Mar 23, 2024

OriolAbril commented Mar 23, 2024

jessegrabowski commented Mar 23, 2024

OriolAbril commented Mar 23, 2024

brandonhorsley commented Mar 23, 2024

ricardoV94 commented Mar 24, 2024

brandonhorsley commented Mar 24, 2024

ricardoV94 commented Mar 24, 2024

ricardoV94 commented Mar 25, 2024 • edited Loading

brandonhorsley commented Mar 25, 2024

ricardoV94 commented Mar 25, 2024 • edited Loading

ricardoV94 commented Mar 25, 2024 • edited Loading

ricardoV94 commented Mar 25, 2024

codecov bot commented Mar 25, 2024 • edited Loading

Codecov Report

welcome bot commented Mar 27, 2024

ricardoV94 commented Mar 27, 2024

brandonhorsley commented Mar 27, 2024

ricardoV94 commented Mar 27, 2024

brandonhorsley commented Mar 22, 2024 •

edited

Loading

ricardoV94 commented Mar 25, 2024 •

edited

Loading

ricardoV94 commented Mar 25, 2024 •

edited

Loading

ricardoV94 commented Mar 25, 2024 •

edited

Loading

codecov bot commented Mar 25, 2024 •

edited

Loading