memory fix for chip-level object down-selection #176
Conversation
There was a problem hiding this comment.
It looks OK, but at least one more person who really loves numpy should look at this since some of the stuff like:
on_chip = np.where(np.logical_or(mag_norm<max_mag,
np.logical_and(xpix>x_min-pix_tol,
np.logical_and(xpix<x_max+pix_tol,
np.logical_and(ypix>y_min-pix_tol,
ypix<y_max+pix_tol)))))
Are things I don't have that much experience with.
| def uss_mem(): | ||
| """Return unique set size memory of current process in GB.""" | ||
| my_process = psutil.Process(os.getpid()) | ||
| return my_process.memory_full_info().uss/1024.**3 |
There was a problem hiding this comment.
I did some research and it looks like this is also supposed to work on OSX:
But it might not be bad to double check this doesn't do something crazy if we aren't on Linux.
There was a problem hiding this comment.
No worries. It works on OSX. (My Mac at least.) And it has both rss and uss attributes as required.
python/desc/imsim/imSim.py
Outdated
| @@ -343,58 +350,77 @@ def sources_from_list(object_lines, obs_md, phot_params, file_name, | |||
| message += " %d had n_points <= 0 \n" % n_bad_knots | |||
| warnings.warn(message) | |||
|
|
|||
| wav_int = None | |||
| wav_gal = None | |||
| target_chips = [target_chip] if target_chip is not None else None | |||
There was a problem hiding this comment.
From the doctoring etc if target_chip is None then you should use all sensors. This is accomplished here by having the target_chips=None interpreted in _chip_downselect as meaning you should use all of them with [det.getName() for det in lsst_camera()].
FWIW I found this confusing when I first read the code. This is far enough done in the function that a comment here and in the docstring of _chip_downselect would probably be useful.
|
The code like this isn't new of course, and so it isn't really part of this PR. |
Yes, I know... I was just saying I struggled a bit with some of the code. There were enough changes in the diff that I went over it all again from scratch. |
| @@ -439,11 +451,8 @@ def sources_from_list(object_lines, obs_md, phot_params, file_name, | |||
| np.logical_and(ypix>y_min-pix_tol, | |||
| ypix<y_max+pix_tol))))) | |||
There was a problem hiding this comment.
FWIW, this is equivalent to the more legible
on_chip = np.where( (mag_norm < max_mag) |
( (xpix > x_min - pix_tol) & (xpix < x_max + pix_tol) &
(ypix > y_min - pix_tol) & (ypix < y_max + pix_tol) )
)
There was a problem hiding this comment.
OK thank you. I
In anycase, I didn't actually find any problems with the code. So, it looks good to me.
|
Unless I hear otherwise, I'll merge this at 12 Noon PT today. |
|
@danielsf I had to update the |
|
Thanks, Scott. I'll merge at 1pm PT today. |
No description provided.