search

adivijaykumar / bilby

MIT License
0 stars 2 forks source link

Error with scipy maximization #8

Closed Kruthi24 closed 3 years ago

Kruthi24 commented 3 years ago

I’m getting a runtime error (screenshot attached) while using scipy differential evolution. I read somewhere that this might have something to do with the callable function (self.log_likelihood_ratio_relative_binning) not being picklable because somehow we might be using multiprocessing. But I don’t know how a picklable function should look like in OOP.

More details: This issue is popping up when we call self.find_maximum_likelihood_waveform to update the fiducial waveform. The scipy differential_evolution function is being called within the function get_best_fit_parameters. https://github.com/adivijaykumar/bilby/blob/9ab7608f537a4634f74a44ee9165662ecffea48b/bilby/gw/likelihood.py#L1585-L1593 Screen Shot 2020-11-04 at 9 52 19 PM

Kruthi24 commented 3 years ago

I've made a temporary fix to this problem. I think the main issue is resolved. We just need to make some further cosmetic changes.

Kruthi24 commented 3 years ago

I think the problem was arising because log_likelihood_ratio function uses self.parameters, and not the parameter values passed by scipy differential_evolution. Hence, for now, I have defined a new likelihood function called lnlike_scipy that takes care of this. Please take a look at https://github.com/adivijaykumar/bilby/commit/ad1bd7c20c947a1c6f96394cdb1decdbfb45e41c.

adivijaykumar commented 3 years ago

Thanks for doing that! I think this is a fix, but it is not the best fix there is. Instead of writing a whole new likelihood function, I think one can just write some kind of wrapper function for the already existing log_likelihood method in the GravitationalWaveTransient class. But this might cause some issues, so I will have to think more about it.