guilgautier / DPPy

Python toolbox for sampling Determinantal Point Processes
https://dppy.readthedocs.io
MIT License
219 stars 52 forks source link

Size of beta-ensembles #29

Closed rbardenet closed 5 years ago

rbardenet commented 5 years ago

Right now, full matrix models have parameter 'N', while banded models have an equivalent parameter called 'size'. Do we want to have two denominations for the size of the matrix?

Naereen commented 5 years ago

It's a good idea. I suggests to pick the most explicit names, and remove one-letter parameters, so chose size or ndim instead of N or n.

guilgautier commented 5 years ago

You're right @rbardenet, that is a bit confusing, since we a dealing with the size N of a matrix, I am tempted to use N but as @Naereen pointed out one-letter parameters are not recommended... Btw, there are also the M and M_1, M_2 sizes for matrix sizes in other models. What about:

Naereen commented 5 years ago

Do as numpy and scipy do:

shape : int or sequence of ints
Shape of the new array, e.g., (2, 3) or 2.

? (cf source)

rbardenet commented 5 years ago

I'd go for 'size', which is the most explicit. I remember reading somewhere that being explicit in the names of your variables never hurts. Re:'shape', it's a good idea, but unless we are extremely careful to always conform to the numpy notation, I think it may bring confusion.

guilgautier commented 5 years ago

Hi @rbardenet and @Naereen, Finally, I've opted for size_* and completely reshape the BetaEnsemble class #34 Check out the Tuto_DPPy or the HTML version for figures 😄