Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fix: Truncated Redshift distribution #59

Merged
merged 2 commits into from
Jul 9, 2024
Merged

Bug fix: Truncated Redshift distribution #59

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df2c4ff
Bug fix: Truncated Redshift distribution
P-1884 Jul 7, 2024
66dc8c6
Adjusting naming conventions
P-1884 Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions paltas/Sampling/distributions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ def __init__(self, z_lens_min,z_lens_mean,z_lens_std,z_source_min,
z_source_mean,z_source_std):
# transform z_lens_min, z_source_min to be in units of std. deviations
self.z_lens_min = (z_lens_min - z_lens_mean) / z_lens_std
self.z_source_min = (z_source_min - z_source_mean) / z_source_std
self.z_source_min_default = (z_source_min - z_source_mean) / z_source_std
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep the original naming convention and instead change the object used in the call

# define truncnorm dist for lens redshift
self.z_lens_dist = truncnorm(self.z_lens_min,np.inf,loc=z_lens_mean,
scale=z_lens_std).rvs
Expand All @@ -355,8 +355,9 @@ def __call__(self):
z_lens = self.z_lens_dist()
clip = (z_lens - self.z_source_mean) / self.z_source_std
# number of std. devs away to stop (negative)
if(clip > self.z_source_min):
if(clip > self.z_source_min_default):
self.z_source_min = clip
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

z_source_min here instead of self.z_source_min would be a cleaner fix. In the current fix a new self.property is created during the call, which doesn't seem like optimal behavior since that property is never referenced outside the call.

else: self.z_source_min = self.z_source_min_default
# define truncnorm dist for source redshift
z_source = truncnorm(self.z_source_min,np.inf,self.z_source_mean,
self.z_source_std).rvs()
Expand Down