lsst-sims / legacy_sims_GalSimInterface

code to seamlessly generate simulated images using GalSim based off of simulated catalogs generated by CatSim
5 stars 7 forks source link

switch back to photon shooting if FFT size is too large #82

Closed jchiang87 closed 5 years ago

jchiang87 commented 5 years ago

add ObjectFlags.unset_flag method to update the rendering info

jchiang87 commented 5 years ago

This is to address a failure mode found during the simulation of the first group of Run2.1i visits, as reported by Antonio:

Found on: node16, node18, node22, node24, node25
Traceback (most recent call last):
  File "/DC2/ALCF_1.2i/scripts/run_imsim.py", line 128, in <module>
    log_level=args.log_level)
  File "/DC2/ALCF_1.2i/scripts/run_imsim.py", line 80, in run_imsim
    image_simulator.run(processes=processes, node_id=node_id)
  File "/DC2/imSim/python/desc/imsim/ImageSimulator.py", line 253, in run
    results.append(simulate_sensor(self.gs_obj_dict[det_name]))
  File "/DC2/imSim/python/desc/imsim/ImageSimulator.py", line 392, in __call__
    fft_sb_thresh=fft_sb_thresh)
  File "/opt/lsst/software/stack/stack/miniconda3-4.5.4-fcd27eb/Linux64/sims_GalSimInterface/2.13.0.sims+2/python/l
sst/sims/GalSimInterface/galSimInterpreter.py", line 939, in drawObject
    gain=detector.photParams.gain)
  File "/DC2/Galsim/galsim/gsobject.py", line 1679, in drawImage
    added_photons = prof.drawFFT(draw_image, add)
  File "/DC2/Galsim/galsim/gsobject.py", line 1889, in drawFFT
    kimage, wrap_size = self.drawFFT_makeKImage(image)
  File "/DC2/Galsim/galsim/gsobject.py", line 1814, in drawFFT_makeKImage
    raise GalSimFFTSizeError("drawFFT requires an FFT that is too large.", Nk)
galsim.errors.GalSimFFTSizeError: drawFFT requires an FFT that is too large.
The required FFT size would be 9958 x 9958, which requires 2.22 GB of memory.
If you can handle the large FFT, you may update gsparams.maximum_fft_size.

An instance catalog that shows this behavior is the one in /global/cscratch1/sd/desc/DC2/Run2.1i/instCat/00677003to00741642/00697974 for sensor R:3,0 S:0,0, and specifically for object 6144000003169 in bulge_gal_cat_697974.txt.gz.

rmjarvis commented 5 years ago

OK, I've looked into the problem object a bit, and I don't think this is the right solution.

The problem is that when offset >> 1, the required FFT size gets really big. Big offsets are easy to do in real space (e.g. photon shooting). But the fft implementation requires using complex phases to implement the shift, which ends up needing a big image in k space to get things to work out right numerically.

So instead, I think we should guard against large offsets, and when we find them, draw into a larger image that has the galaxy near the center of the image, and then copy over just the overlapping part into the original image.

See this gist for how I imaging doing this.

I'll work up a PR that does this in a general way. My notebook hardcodes the fact that offset.x >> 1. We'd want to do a similar thing if either x or y is large (or both), and positive or negative.

rmjarvis commented 5 years ago

OTOH, I think we can keep the changes here, and just add the offset/bounds bit before the try block. That way, if it still fails (which presumably would be even more rare in that case, but maybe still possible), then we can still fall back to photon shooting.

So, my question is, should I work on the u/jchiang/large_fft_size_handling branch and add my part to this PR? Or start a new branch with a separate PR?

jchiang87 commented 5 years ago

It seems easiest just to add to this branch.

rmjarvis commented 5 years ago

Done. 74ab720445e548547c0d2d958850c3065c562c99

jchiang87 commented 5 years ago

Ok, I ran the updated code with 74ab720 (and 097cc58) and the galaxy at issue was rendered using the FFT according to the centroid file, and using fc10e30 (prior to Mike's changes), the object used photon shooting. So it appears that everything is working as intended. I'll do another run with the full instance catalog in case some unforeseen issues are lurking. If the new image looks consistent with my earlier run with fc10e30, I'll consider this good to go and will merge this evening at 9pm PT, unless I hear otherwise.