search

PAHdb / pyPAHdb

A Python tool to decompose astronomical PAH emission into contributing PAH subclasses.
https://www.astrochemistry.org/pahdb/
BSD 3-Clause "New" or "Revised" License
7 stars 6 forks source link

Possible performance issues? #15

Closed mattjshannon closed 6 years ago

mattjshannon commented 6 years ago

Qualitatively, running tutorial 1 (with the API changes of decomposer.py) seems like it might have lost some performance. @PAHdb, would you be able to benchmark pyPAHdb using the same data/parameters as was done for the conference proceeding? Just to see if the numbers still line up. Thanks.

PAHdb commented 6 years ago

The issue is having all the multiprocessing-functions inside the class. You should move them outside the class again, i.e., _decomposer_interp, etc. Don't forget to remove the 'self'.

mattjshannon commented 6 years ago

The issue is having all the multiprocessing-functions inside the class. You should move them outside the class again, i.e., _decomposer_interp, etc. Don't forget to remove the 'self'.

Interesting, do you know why that causes a performance difference? Naively I wouldn't have expected it. Will correct this tomorrow, thanks!

PAHdb commented 6 years ago

I haven't confirmed this, but my suspicion would be that the class-references need to be resolved each time in the main thread, i.e., the 'self' in Array(self, function) needs to be resolved on each call, while having it outside the class avoids needing to 'find' self each time.

mattjshannon commented 6 years ago

I haven't confirmed this, but my suspicion would be that the class-references need to be resolved each time in the main thread, i.e., the 'self' in Array(self, function) needs to be resolved on each call, while having it outside the class avoids needing to 'find' self each time.

I think you must be right. That fixed the issue. I'll bundle it up with some other improvements/fixes later (likely improving the unit tests and whatever else is on the to-do list).

This reminds me that we should add a TODO to the list: code profiling. We might be able to optimize its operation further. I'll add an issue to this effect.

PAHdb commented 6 years ago

Pull Request #17 fixes the issue.