toy simulator that is 11,000 times faster than galsim #961
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #961 +/- ##
==========================================
+ Coverage 92.50% 92.69% +0.19%
==========================================
Files 23 24 +1
Lines 2934 3041 +107
==========================================
+ Hits 2714 2819 +105
- Misses 220 222 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
|
||
| # make predictions/inferences | ||
| target_cat1 = target_cat.get_brightest_sources_per_tile(band=2, exclude_num=0) |
There was a problem hiding this comment.
Nice, this is a helpful fix/generalization
|
|
||
|
|
||
| @dataclasses.dataclass | ||
| class ToySimulator: |
There was a problem hiding this comment.
Could we inherit from pl.LightningDataModule and IterableDataset here, for symmetry with SimulatedDataset? Ideally ToySimulator would be as close to a drop-in replacement for SimulatedDataset as possib.e
There was a problem hiding this comment.
Oh maybe this is OK, I see now it's ToySimulatedDataset that's intended as the drop-in replacement for SimulatedDataset. It does seem a bit cleaner to me to merge ToySimulator and ToySimulatedDataset, and to inherit from the same classes that SimulatedDataset does.
| return torch.stack(band_images, dim=1) | ||
|
|
||
|
|
||
| class ToySimulatedDataset(IterableDataset): |
There was a problem hiding this comment.
Maybe clearer to merge this class with ToySimulator, for symmetry with SimulatedDataset. (The reason we have two classes in simulated_dataset.py is that the second reads from disk.)
| return range_tensor < expanded_counts | ||
|
|
||
|
|
||
| def draw_trunc_pareto(b, c, loc, scale, size, torch_generator=None): |
There was a problem hiding this comment.
We used to have code like this in prior.py but now we use scipy.stats.truncpareto.
Would it be possible to have your simulator take a prior as argument, the way SimulatedDataset does, for greater symmetry? Presumably you'd just ignore some of the fields for now (e.g., source type). That would make this more of a drop-in replacement for SimulatedDataset, which I think is the goal, at least if we're including this code in our main bliss/bliss directory (as opposed to a case study).
| * ( | ||
| (plocs_x - xx) ** 2 / self.psf_radius_h**2 | ||
| + (plocs_y - yy) ** 2 / self.psf_radius_w**2 | ||
| ) # noqa: C815 |
There was a problem hiding this comment.
black formatter strikes again. it's cleaner to use intermediate variables.
((Also some tiny bug fixes where it was assumed that the reference band should always be 2.))