Evolving MMBulge Amplitude #94
Conversation
Rewrite mass_stellar() to reshape arrays within the function rather than afterwords. Also changed lines in static_binary_density() and gwb_ideal() to remove reshaping of arrays after mass_stellar() is called since it is now performed in the function
Added redz argument to non-evolving versions of the MMBulge scaling relations. Default is redz=1, since redz is not used in the non-evolving versions of the functions it doesn't matter what the default/input is
Added redz argument to instance of mbh_from_host(). Default is redz=1
Added missing redz argument to mbh_from_host
Removed line defining redz in the wrong location
Added try except statement to handle array shaping in redshift-dependent version of mbh_from_mbulge. There may be a better way to do it, but this certainly works
|
@lzkelley All the tests passed so I believe this is now good to go. I did add a try-except statement in right at the last minute to fix an issue that was popping up in the host-relations notebook. I don't know if that was the most efficient way to solve that issue (maybe the notebook was wrong?), so feel free to change it if it's obvious or want me to take some more time to think about a more clever solution let me know. |
holodeck/population.py
Outdated
| @@ -600,7 +600,7 @@ class PM_Mass_Reset(_Population_Modifier): | |||
| """Reset the masses of a target population based on a given M-Host relation. | |||
| """ | |||
|
|
|||
| def __init__(self, mhost, scatter=True): | |||
| def __init__(self, mhost, redz=1, scatter=True): | |||
There was a problem hiding this comment.
It looks like this redz parameter is unused. What's the purpose of including it here, and setting to a default of 1?
holodeck/population.py
Outdated
| @@ -633,7 +633,7 @@ def modify(self, pop): | |||
| # Store old version | |||
| pop._mass = pop.mass | |||
| # if `scatter` is `True`, then it is set to the value in `mhost.SCATTER_DEX` | |||
| pop.mass = self.mhost.mbh_from_host(pop, scatter) | |||
| pop.mass = self.mhost.mbh_from_host(pop, redz=pop.redz, scatter=scatter) | |||
There was a problem hiding this comment.
Because the whole pop instance is being passed to the function, the function itself can always grab pop.redz. Why include it as an argument also?
| @@ -351,7 +351,7 @@ def mbh_from_mstar(self, mstar, scatter): | |||
| mbulge = self.mbulge_from_mstar(mstar) | |||
There was a problem hiding this comment.
The redz parameter should be passed through here also, into the mbulge_from_mstar and mbh_from_mbulge functions, right?
holodeck/sams/sam.py
Outdated
| @@ -191,7 +191,7 @@ def shape(self): | |||
| self._shape = tuple([len(ee) for ee in self.edges]) | |||
| return self._shape | |||
|
|
|||
| def mass_stellar(self): | |||
| def mass_stellar(self, redz): | |||
There was a problem hiding this comment.
This is fine, but it would probably be a little cleaner to just grab the redz values within this function, instead of passing them in. i.e. within this function:
redz = self.redz[np.newaxis, np.newaxis, :]
The motivation being that there are no other redshift values that would be used (at least not that I can think of).
| mstar_rat = mstar_tot / mstar_pri | ||
| # M = m1 + m2 | ||
| mstar_tot = mstar_pri + mstar_tot | ||
|
|
There was a problem hiding this comment.
Nice, I like moving all of this into the mass_stellar() function!
Calculate redz in mass_stellar() rather than calling as argument
Changed all instances of redz=1 to redz=None. Also updated mbh_from_host() to determine redz from pop rather than from an argument
Removed redz as arguments in two functions since they were unused.
Fixing redz arguments, removed something that was added in error
Removed argument redz from mbh_from_host() in line 107
Description
This update allows usage of "...Redshift..." versions of MMBulge functions in relations.py
Previously evolving relations only worked if shape=N, now they run for shape=None
The biggest change was to mass_stellar() in sam.py
Previously the output of this function was reshaped after call, now arrays are reshaped in the function and returned with shape (M, Q, Z)
Todos
Notable points that this PR has either accomplished or will accomplish.
Questions
Status