ENH: Add DiscreteGuideTable wrapper

[Kai-Striega]

Reference issue

gh-14600

What does this implement/fix?

This is a very minimal start of to the DiscreteGuideTable wrapper. It is included in all_methods and the associated tests pass, however additional tests, documentation and type hinting must still be added.

Additional information

I'm having some problems having the wrapper recognize the probability vector memory view, this can be reproduced as follows:

import numpy as np
from scipy.stats import DiscreteGuideTable

pv = [0.1, 0.3, 0.6]
urng = np.random.default_rng()
rng = DiscreteGuideTable(pv, random_state=urng)
Traceback (most recent call last): File "", line 1, in File "unuran_wrapper.pyx", line 1632, in scipy.stats._unuran.unuran_wrapper.DiscreteGuideTable.__cinit__ AttributeError: 'scipy.stats._unuran.unuran_wrapper.DiscreteGuideTable' object has no attribute 'pv_view'

It's also interesting to note that the tests still pass despite this.

[chrisb83]

Hi Kai, thanks for working on this. Don't forget to add the type hints in the pyi-file in the _unuran folder.

[tirthasheshpatel]

Sorry for all the conflicts, @Kai-Striega! I have resolved them for you. If you have changed something locally, I think you can save it in some other temporary workspace or stash the changes using git stash. After pulling the changes here, you can apply your local changes again. Sorry if this is too much trouble for you!

[Kai-Striega]

EDIT: Fixed by rebuilding all files from source.

I'm having some problems with importing DiscreteGuideTable after your changes, could you take a quick look if it's something obvious?

I haven't made any relevant changes so I suspect it's introduced by the code you merged, although I'm not 100% sure.

[tirthasheshpatel]

Some comments on typos, style, and tests. Wrapper code, docs, and tests look very good! You can copy-paste the documentation in the tutorial and write a small benchmark for the method to finish this up. Given the similarity to DiscreteAliasUrn, I think we should be good to go if tuts and benchmarks look good. Can you take a look whenever you are free @chrisb83?

[Kai-Striega]

@tirthasheshpatel could you take a look at why there are merge issues. I don't think that it's anything I did but I'm not sure what's causing them

[tirthasheshpatel]

Thanks for the changes, @Kai-Striega! I think the last thing left to do is return domain[0]-1 when ppf is evaluated at u = 0. Also, can you add the ppf example to the tutorial? Don't worry about the conflicts. We can resolve them before merging.

[Kai-Striega]

I think the last thing left to do is ...

There's also one more instance of guide_factor that requires back-ticks around it

return domain[0]-1 when ppf is evaluated at u = 0

I'm having some weird issue with this. I've added the line self.domain = domain on line 2101 but, even without any changes to ppf I'm getting the error AttributeError: 'scipy.stats._unuran.unuran_wrapper.DiscreteGuideTable' object has no attribute 'domain'. I'm going to try an rebuild it from scratch to see if that helps otherwise it may take some time to figure out why I can't add that attribute.

Also, can you add the ppf example to the tutorial?

This should already be done starting on line 102 of doc/source/tutorial/stats/sampling_dgt.rst

Don't worry about the conflicts. We can resolve them before merging.

ok. I just don't normally like not having the tests run

[tirthasheshpatel]

I just don't normally like not having the tests run

Oh, yes! Let me know once you are done with the domain change. I will help resolve conflicts afterwards.

[Kai-Striega]

@tirthasheshpatel it should be ready now ;)

[tirthasheshpatel]

This is very close! Last batch of comments to resolve the failing tests.

[Kai-Striega]

@tirthasheshpatel I've fixed the CI failure test failure, there is one more on MyPy that I can't fix - short of removing the line in its entirety. Could you please take a look and check if there is any better way to get it to pass?

[tirthasheshpatel]

Everything looks good to me now! Tests are comprehensive; docs and tutorials are informative. Only one comment on the tutorial above. @chrisb83 Let me know if we still have your approval after the new changes. If you are happy with it, feel free to merge!

[Kai-Striega]

@tirthasheshpatel @chrisb83 I'd like to thank both of you for all the help - and patience - on this PR. It is appreciated!

[tirthasheshpatel]

Thanks for your help too @Kai-Striega! This was a big one :)

[chrisb83]

Thanks Kai and Tirth! Nice to have this method in SciPy now

[tirthasheshpatel]

Thanks again @Kai-Striega! And thanks @chrisb83 for the reviews!