Work on cluster lensing, add LOS lens draw method to lens_pop.py, optimize _image_position_from_source for multi_plane="Source" lenses#434
Conversation
…tes an entire field of source galaxies rather than only returning a single source Optimized Lens._image_position_from_source for multi_plane="Source" lenses Added option to have the Lens class automatically move source galaxies in front of deflector to field galaxieswq
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
simonjs830
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #434 +/- ##
==========================================
- Coverage 98.65% 98.07% -0.59%
==========================================
Files 106 106
Lines 8340 8398 +58
==========================================
+ Hits 8228 8236 +8
- Misses 112 162 +50
🚀 New features to boost your workflow:
|
sibirrer
left a comment
sibirrer
left a comment
There was a problem hiding this comment.
Thank you @simonjs830!
I made some comments. You also see that you did not add tests. Let me know if you have questions!
| z_source_convention=self.max_redshift_source_class.redshift, | ||
| use_jax=use_jax, | ||
| multi_plane=bool(self.multi_plane), | ||
| multi_plane=bool(self.multi_plane) if multi_plane is None else multi_plane, |
There was a problem hiding this comment.
The code works. Perhaps it is better to see how self.multi_plane is initialized and adapt that instead of additional criteria here
| while True: | ||
| # draw random deflector | ||
| _deflector = self._lens_galaxies.draw_deflector() | ||
| cluster_id = _deflector._deflector._deflector_dict["cluster_id"] |
There was a problem hiding this comment.
this line of code seems only to work for clusters and accesses a private variable. This should be avoided. Do you need that?
| _deflector = self._lens_galaxies.draw_deflector() | ||
| cluster_id = _deflector._deflector._deflector_dict["cluster_id"] | ||
|
|
||
| ### compute smallest square containing all deflector subhalos, to avoid computing unnecessary caustics and source galaxies |
There was a problem hiding this comment.
this feature to calculate the influence area where to check for sources behind should be generalized and be part of the the Deflector() class
| bbox = [[np.inf, -np.inf], [np.inf, -np.inf]] | ||
| max_dist = 20 | ||
|
|
||
| for s in _deflector._deflector._subhalos: |
There was a problem hiding this comment.
again, access to a private variable that may not be generally supported
| if self._cached_caustics[cluster_id] == -1: | ||
| continue | ||
|
|
||
| ra_caustic_list, dec_caustic_list = self._cached_caustics[cluster_id] |
There was a problem hiding this comment.
I see that you cache here the caustics for individual deflectors/clusters. Not sure this is needed, or you can generalize it to deflector IDs. The danger here is that this will use up a lot of memory when having many (potential) deflectors. Do you need to save the caustics or do you only need the area?
| use_jax=self._use_jax, | ||
| ) | ||
| lens_model_object, model_params = ( | ||
| _dummy_lens.deflector_mass_model_lenstronomy() |
There was a problem hiding this comment.
there might be a way that you don't need a dummy Lens() instance. You need the deflector model for a high-redshift source only
| center_y=center[1], | ||
| compute_window=2 * radius, | ||
| # compute_window = 150, | ||
| grid_scale=2.0, # 0.5, #TODO experiment with grid_scale |
There was a problem hiding this comment.
with a 2 arc second grid you would not see images appearing closer than 2" together. I know this is expensive and might not be needed to quantify whether there are at least two images.
| (ra_caustic_list - s.extended_source_position[0]) ** 2 | ||
| + (dec_caustic_list - s.extended_source_position[1]) ** 2 | ||
| ) | ||
| < 5**2 |
There was a problem hiding this comment.
I don't understand that part
| source_class=_source_cut, | ||
| cosmo=self.cosmo, | ||
| use_jax=self._use_jax, | ||
| multi_plane="Source", |
There was a problem hiding this comment.
this might not be as general
|
View / edit / reply to this conversation on ReviewNB sibirrer commented on 2026-06-22T20:06:24Z Line #1. import slsim.Sources as sources add a markdown sell above with a title, and a short description of what it does |
|
View / edit / reply to this conversation on ReviewNB sibirrer commented on 2026-06-22T20:06:25Z Line #2. import slsim.Pipelines as pipelines would be nice to have a bit more documentation of the code cells |
|
View / edit / reply to this conversation on ReviewNB sibirrer commented on 2026-06-22T20:06:25Z Line #1. cluster_catalog = Table.read("../data/redMaPPer/clusters_example.fits")
describe this catalogue |
|
View / edit / reply to this conversation on ReviewNB sibirrer commented on 2026-06-22T20:06:26Z Line #1. def debug_plot_objects(lens_cluster_class: Lens, ax): remove "debug" |
|
View / edit / reply to this conversation on ReviewNB sibirrer commented on 2026-06-22T20:06:27Z Line #1. # for (lens_cluster_class, image_rgb) in saved_renders.keys(): can you label the axis in the plots (i.e. arcsec) |
2 participants
No description provided.