GalSim-developers / GalSim

The modular galaxy image simulation toolkit. Documentation:
http://galsim-developers.github.io/GalSim/
Other
225 stars 106 forks source link

Variable postage stamp sizes #391

Closed dkirkby closed 11 years ago

dkirkby commented 11 years ago

I am trying to generate Scattered images from deep catalogs and running into two issues related to the large range of source magnitudes in my catalog. If I generate each postage stamp with stamp_size == size, then I get the correct results (except for some ringing in the tails - is there an accuracy parameter to control this?) but this is very slow. It would obviously be smarter to use a different stamp size for each source, so I tried stamp_size = 0, but this truncates the flux at O(1e-3) of the peak flux, which means that the blending of the tails of bright objects with the cores of much fainter objects is not treated correctly. This leads to my feature request:

Another related issue is that since automatic sizes might be even or odd, about half of my sources are rendered with a half pixel offset because of Issue #380.

After calculating per-object sizes myself, I ran into a new issue:

Specifically, the following snippet does not generate a syntax error but only uses the stamp size for the first object in the catalog:

stamp_xsize: { type: Eval, str: '2*int(math.ceil(dx/scale))+1', fdx: { type: InputCatalog, col: 6 } }
stamp_ysize: { type: Eval, str: '2*int(math.ceil(dy/scale))+1', fdy: { type: InputCatalog, col: 7 } }

As a workaround, I am now generating individual Single images with varying sizes that I then stack myself as a post processing step, instead of rendering directly to a Scattered image.

rmandelb commented 11 years ago

On Apr 11, 2013, at 6:29 AM, dkirkby notifications@github.com wrote:

I am trying to generate Scattered images from deep catalogs and running into two issues related to the large range of source magnitudes in my catalog. If I generate each postage stamp with stamp_size == size, then I get the correct results (except for some ringing in the tails - is there an accuracy parameter to control this?)

We have a number of parameters that determine accuracy, but for the version of the code on the master branch, they have to be changed by recompiling. We have a pull request in now ( https://github.com/GalSim-developers/GalSim/pull/385 ) for some work that made it possible to change the accuracy parameters on the fly, so that might be helpful for you eventually. I'm guessing it's one of the Fourier space params that is the issue here.

However, we also have a few open issues that are for investigating numerical accuracy, so we might adjust the defaults. if you have some specific code snippet that illustrates this ringing, I would be interested in seeing it, since it could help us make decisions about these defaults.

but this is very slow. It would obviously be smarter to use a different stamp size for each source, so I tried stamp_size = 0, but this truncates the flux at O(1e-3) of the peak flux, which means that the blending of the tails of bright objects with the cores of much fainter objects is not treated correctly.

As a short-term solution, you could call the draw command with the wmult keyword set to something larger than 1, so that GalSim will automatically decide on a size for each image and then increase its size by wmult. This leads to my feature request:

Could there be an automatic stamp sizing option that includes pixels down to a specified surface brightness (or pixel ADU) threshold? Interesting. Chihway mentioned that she had already run into some issues with this for extremely bright sources (which would be saturated stars) for which our "must contain x% of the flux" criterion is inadequate to represent most of the object. Of course, most users will not be trying to draw stars that would be saturated, but it's still an issue even for very bright galaxies.

In principle, I think what you're suggesting would be easy to code up - even easier than the current way of choosing an image size. I'm willing to do this, unless somebody else wants to do it or has another way to solve this issue that doesn't involve adding a new option.

Also, once the pull request I linked above gets merged into master, you will be able ot change the "x% of the flux" criterion for automatical image sizes on the fly, since it comes from the value of "alias_threshold".

Another related issue is that since automatic sizes might be even or odd, about half of my sources are rendered with a half pixel offset because of Issue #380.

Well, it would also be super easy to simply choose that good image sizes can be always even / always odd. But let's see what happens with #380. After calculating per-object sizes myself, I ran into a new issue:

I'll leave that for Mike (who might be busy at the DES meeting).

dkirkby commented 11 years ago

Could there be an automatic stamp sizing option that includes pixels down to a specified surface brightness (or pixel ADU) threshold? Interesting. Chihway mentioned that she had already run into some issues with this for extremely bright sources (which would be saturated stars) for which our "must contain x% of the flux" criterion is inadequate to represent most of the object. Of course, most users will not be trying to draw stars that would be saturated, but it's still an issue even for very bright galaxies.

I estimate that you need a source magnitude range larger than 7 to be affected by this problem (and you also need to care about the effect of bright tails on faint sources), but the LSST simulation galaxy catalog I am using covers r(AB) from 16 to 28, so I am well over this threshold without any bright stars!

In principle, I think what you're suggesting would be easy to code up - even easier than the current way of choosing an image size. I'm willing to do this, unless somebody else wants to do it or has another way to solve this issue that doesn't involve adding a new option.

Also, once the pull request I linked above gets merged into master, you will be able ot change the "x% of the flux" criterion for automatical image sizes on the fly, since it comes from the value of "alias_threshold".

It sounds like the ideal solution would be to somehow generalize the new "alias_threshold" to specify either a minimum enclosed flux fraction, or else a minimum pixel ADU threshold.

rmjarvis commented 11 years ago

It sounds like the ideal solution would be to somehow generalize the new "alias_threshold" to specify either a minimum enclosed flux fraction, or else a minimum pixel ADU threshold.

The alias_threshold parameter is the converse of this. It is the maximum flux fraction that is allowed to be not enclosed. The default isn't all that low currently -- 5.e-3 -- so this is probably the underlying problem for your case.

It does not seem possible to provide values for the Scattered image stamp_size from an input catalog. Could this feature be added?

Without looking at the code right now, I think the problem here is that the "sequence index" for this is wrong. I'm guessing that it must be using the image number rather than the object number. That will be easy to fix. But probably not until next week at least. I'm pretty busy with DES stuff this week. I'll also attack your image centering problems then.

dkirkby commented 11 years ago

It sounds like the ideal solution would be to somehow generalize the new "alias_threshold" to specify either a minimum enclosed flux fraction, or else a minimum pixel ADU threshold.

The alias_threshold parameter is the converse of this. It is the maximum flux fraction that is allowed to be not enclosed.

i.e., 1 - (min enclosed flux fraction) ?

The default isn't all that low currently -- 5.e-3 -- so this is probably the underlying problem for your case.

My underlying problem wouldn't easily be solved with a lower threshold (even if it was possible to specify a threshold per object) since what I really want is a threshold based on the absolute sky brightness instead of the brightness relative to the total source flux.

It does not seem possible to provide values for the Scattered image stamp_size from an input catalog. Could this feature be added?

Without looking at the code right now, I think the problem here is that the "sequence index" for this is wrong. I'm guessing that it must be using the image number rather than the object number. That will be easy to fix. But probably not until next week at least. I'm pretty busy with DES stuff this week. I'll also attack your image centering problems then.

Thanks. I have work arounds for now so there is no rush.

rmjarvis commented 11 years ago

The alias_threshold parameter is the converse of this. It is the maximum flux fraction that is allowed to be not enclosed. i.e., 1 - (min enclosed flux fraction) ?

Exactly.

what I really want is a threshold based on the absolute sky brightness instead of the brightness relative to the total source flux.

That would be a lot trickier to implement. However, you might look into using the photon shooting methods for the faint objects. Even if they have a strict alias_threshold, their computing time would still just scale with the number of photons shot. So that would probably be a lot faster while still having the accuracy you need.

rmjarvis commented 11 years ago

The items in this issue have been addressed by #365 and #380, both of which are now merged into master. Closing.