search

astropy / halotools

Python package for studying large scale structure, cosmology, and galaxy evolution using N-body simulations and halo models
http://halotools.rtfd.org
BSD 3-Clause "New" or "Revised" License
104 stars 65 forks source link

Eliminate independently-defined LogNormalScatter class #746

Open aphearin opened 7 years ago

aphearin commented 7 years ago

Defining scatter in an independent class is a bit convoluted because the functionality is so simple. After some experimenting with building models, reliance on this class does not really improve convenience. It is probably better to entirely do away with this class and just have any stellar-to-halo-mass relation implement its own scatter. The downside is a mild amount of code duplication will be required, but I think this outweighed by the improved clarity and relaxed restrictions on future development.

It may be better to similarly refactor the stellar-to-halo-mass relations so that PrimGalPropModel is also entirely eliminated. @johannesulf or @duncandc - do either of you have an opinion on this? If it makes no difference to either of you, I'll probably just go ahead and eliminate both of these classes.

johannesulf commented 7 years ago

I have no strong opinion on that. Generally, I agree with you on that issue. I didn't even know that such a class existed and built models without it.

duncandc commented 7 years ago

I 100% agree with you both on this issue. In fact, I have already ditched the scatter class when building my own models. This is certainly a case of us being a little too class happy in the early days of halotools.

aphearin commented 7 years ago

Great, thanks for weighing in. I'll definitely get rid of the scatter class.

I'll take a close look at getting rid of PrimGalpropModel class as well. The main feature that PrimGalpropModel had was to automatically provide the mc_ version of some mean relation such as <M*|Mh> defined by the user. In hindsight, that machinery seems unnecessary (and actually obfuscating).