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

Add option to generate a RandomWalk GalSim component #39

Closed EiffL closed 6 years ago

EiffL commented 6 years ago

@danielsf @jchiang87 For the purpose of generating more realistic looking galaxies, we would like to be able to make use of the RandomWalk light profile introduced in the latest 1.5 version of galsim. I'm happy to implement the necessary code, by quickly looking at the GalSimInterface it seems like I might just need to add a "GalSimRandomWalk" class in galSimCatalogs.py. Am I missing something ?

danielsf commented 6 years ago

Yes. You probably just need to inherit from GalSimGalaxies, but change this line

https://github.com/lsst/sims_GalSimInterface/blob/master/python/lsst/sims/GalSimInterface/galSimCatalogs.py#L572

so that it sets galsim_type to something like random_walk_galaxy. You would then need to add a drawRandomWalk() method to the GalSimInterpreter class; this method would need to be analogous to the drawSersic() method that actually draws Sersic profiles, defined here

https://github.com/lsst/sims_GalSimInterface/blob/master/python/lsst/sims/GalSimInterface/galSimInterpreter.py#L355

Finally, GalSimInterpreter.createCenteredObject() would need to know what to do with galsim_type=='random_walk_galaxy'; see

https://github.com/lsst/sims_GalSimInterface/blob/master/python/lsst/sims/GalSimInterface/galSimInterpreter.py#L382

jchiang87 commented 6 years ago

I think we may need some way to connect the properties of the RandomWalk component to its parent galaxy, if the flux from the parent is shared between the normal bulge/disk components and these RandomWalk entries. Not sure where the code to implement that should go since it implies an additional layer of hierarchy as the bulge and disk components are currently generated as if they are independent, see https://github.com/LSSTDESC/sims_GCRCatSimInterface/blob/master/python/desc/sims/GCRCatSimInterface/InstanceCatalogWriter.py#L113

danielsf commented 6 years ago

Oh. Sorry. I thought the RandomWalk galaxy was going to be something in place of the Sersic profiles. Layering it on top of them will be more tricky, as Jim says.

EiffL commented 6 years ago

Yes, I was thinking the random walk would be an independent component.

jchiang87 commented 6 years ago

So we'd have a RandomWalk db to query then?

EiffL commented 6 years ago

@jchiang87 Based on your script to generate the imsim instance catalog, I was just thinking of plugging a function in there that would derive the appropriate flux and properties from the bulge and disk catalogs, then reduce the flux in the disk and add a random walk component in the output instance catalog

jchiang87 commented 6 years ago

I think that's probably fine for the instance catalogs, but I'm wondering if there is a need to access the RandomWalk galaxy properties outside of that context, e.g., via gcr-catalogs.

danielsf commented 6 years ago

I apologize if this goes too far into the weeds, but:

The GalSim interface treats bulges and disks in two separate steps because that is how the database underlying CatSim was designed. Since DC2 is not going to get the galaxies from a catalog on disk, we could probably create a new galsim_type = compound_galaxy that will simultaneously draw the bulge and the disk (and, presumably, the random walk component), and eliminate the need to query the DC2 catalog twice.

EiffL commented 6 years ago

@jchiang87 Yeah, they wouldn't be accessible.... How about that then, I can add a gcr-catalog reader function that will produce the parameters of the random walk on the fly from the rest of the DC2 catalog, while also adding a new column for the corrected flux of the disc component.

@danielsf I guess it makes sense, but I gues that involves a lot of modifications at different levels

danielsf commented 6 years ago

@EiffL I actually don't think we would have to do too much modification to the GalSim interface to do what I am proposing. Just add a drawCompound method to GalSimInterpreter; add a class to GalSimCatalogs that knows it will need bulge, disk, and random walk components; and add a database-like class to this file

https://github.com/LSSTDESC/sims_GCRCatSimInterface/blob/master/python/desc/sims/GCRCatSimInterface/DatabaseEmulator.py

that knows how to map the output of the gcr reader into something the new GalSimCatalog class expects.

EiffL commented 6 years ago

Hum... Yes, I like it, could it also translate into some speed up ? I don't know what the overhead is of calling the GalSimInterpreter multiple times to draw the multiple components of a single galaxy.

My concern was more about having divergences between the ways ImSim and PhoSim are going to handle an instance catalog. I'm assuming the reason why AGN, bulge and disks are separate in the instance catalog has something to do with how PhoSim works (which I have no clue about). That's why I was thinking just adding extra RandomWalks entries at the end of the instance catalogs was harmless but grouping all the components of the galaxy together would change the philosophy of the instance catalogs.

jchiang87 commented 6 years ago

Unfortunately, for phosim, I think the only way to simulate the RandomWalk galaxies is as collection of point sources. So it would be good to have an option to write out those objects in that form.

danielsf commented 6 years ago

Right. I do keep forgetting that we want ImSim and PhoSim to have the same user interface.

cwwalter commented 6 years ago

So I thought our decision was that the two simulations didn't need to be the same and what we could do at least for now was basically set a switch where the interface told GalSim that it should use the random walk. In other words, we wouldn't write them into the instance catalog but we would let GalSim do the work. So that should be quite fast to implement for DC2. Is the proposal here to let GalSim do the work at the catalog level instead?

If we adhere to making them a component in the instance file you would need to add a point (basically a star) for every knot layered for every object. I think that would be a non-negligible increase in the instance catalog size right? How many knots are there on average per galaxy?

EiffL commented 6 years ago

Yes, so the problem is that you want to know both bulge and disk components to get the parameters of the RandomWalk, at the moment, these are generated as completely independent things by ImSim. So you need to decide the parameters of the random walk, at least at the level of the creation of the instance catalog, or before that (as part of the descqa catalog reader).

I've just added an option for the GalSimInterface to draw a randomwalk, as another type of objects, alongside sersic profiles and point source.

My proposal is this, at the level of the creation of the instance catalog, the phosim catalog contains only the sersic profiles, agn and stars. And the ImSim instance has an additional set of entries (one per galaxy) to draw a RandomWalk profile, in addition to the previous elements. So, the instance catalog doesn't have to store all the knots, only the parameters that will let GalSim randomly draw them.

So, the switch would be at the level of the creation of the ImSim instance catalog, which could either be identical to ImSim, or contain additional RandomWalk light profiles.

Does that sound good ?

cwwalter commented 6 years ago

Yes, that sounds reasonable to me for now. We need to make the imSim parser read a new type of object so we should track an issue for that too (or we could put it in LSSTDESC/imSim#79).

In the longer term LSSTDESC/imSim#71 is our plan for a future more flexible binary instance catalog format. So, we can think about what the "ideal" description of a galaxy would be there.

cwwalter commented 6 years ago

Oh, @EiffL also note LSSTDESC/imSim#19.

EiffL commented 6 years ago

I have opened this pull request to add support for drawing a random walk light profile: https://github.com/lsst/sims_GalSimInterface/pull/40

The modifications are minimum, reusing most of the GalSimGalaxies structure and parameters, just changing the meaning of the sindex parameter which in the case of a random walk light profile becomes the number of knots. I initially wanted to introduce an additional parameter n_knots specific to the RandomWalk but that created a lot of side effects...

This code requires at least GalSim 1.5 to use the RandomWalk light profile.

cwwalter commented 6 years ago

This can be closed now right?

cwwalter commented 6 years ago

@EiffL can this be closed?

EiffL commented 6 years ago

sure, I'll go ahead and close it