keflavich / imf

Simple tools to work with the Initial Mass Function
MIT License
44 stars 17 forks source link

mmin and mmax are not correctly passed by inverse_imf #13

Closed smeingast closed 4 years ago

smeingast commented 5 years ago

Hey there. When I call inverse_imf with the arguments mmin or mmax, the created mass function instance does not use these values. For instance

inverse_imf(np.random.random(10000000), massfunc="kroupa", mmin=0.01, mmax=200.0)

still returns a minimum mass of 0.03 and a maximum of 120. ;)

keflavich commented 5 years ago

uh oh.... that looks like a bug.
image is correct.

This should be easy to fix; we just need to pass those parameters onto the massfunc correctly.

In the meantime, you can probably work around w/inverse_imf(..., massfunc=imf.Kroupa(mmin=0.01, mmax=200.0))

smeingast commented 5 years ago

Indeed and thanks for the quick response. 🙂 I already have a workaround going here locally, but I will update my repo once you implemented the fix. Thanks!!

keflavich commented 4 years ago

eb479c5 may have solved this: I now use rvs directly in inverse_imf

wbuthod commented 4 years ago

I'm having a similar issue. For example:

import imf, random
nmin,nmax = 201.0, 0.0
while True:
    m = imf.inverse_imf([random.random()],massfunc=imf.Kroupa(mmin=0.01,mmax=200))[0]
    if m < nmin:
        nmin=m
        print(nmin,nmax)
    if m > nmax:
        nmax=m
        print(nmin,nmax)

After some run time, the results still converge at 0.03 and 120.0

keflavich commented 4 years ago

A much simpler test is imf.inverse_imf(1,massfunc=imf.Kroupa(mmin=0.01,mmax=200))

In this case, the problem is that inverse_imf is overriding the mmin/mmax. Instead, try: imf.inverse_imf(1,massfunc=imf.Kroupa(), mmin=0.01,mmax=200) i.e., pass mmin/mmax to inverse_imf instead of to imf.Kroupa.

This is not great UI design; I'll see if there's a simple fix...

wbuthod commented 4 years ago

That appears to work for mmax, but not mmin:

>>> imf.inverse_imf([0,1],massfunc=imf.Kroupa(),mmin=0.01,mmax=200)
array([2.99050745e-02, 1.99013557e+02])
>>> imf.inverse_imf([0,1],massfunc=imf.Kroupa(),mmin=0.01,mmax=60)
array([2.99163452e-02, 5.97398875e+01])
keflavich commented 4 years ago

uh, hmm. It should. This might be a deeper problem. I did just push a commit fixing the UI though

keflavich commented 4 years ago

I'm actively debugging this, but I'm introducing some bugs along the way... =(

wbuthod commented 4 years ago

Works for me now. Thanks!

>>> import imf
>>> print(imf.inverse_imf([0,1],mmin=0.01,mmax=120))
[1.0047232e-02 1.1943852e+02]
>>> print(imf.inverse_imf([0,1],mmin=0.02,mmax=10))
[0.0200624 9.9689924]
>>>>
keflavich commented 4 years ago

Great, closing! Raise a new issue if this comes back up