LSSTDESC / sims_GCRCatSimInterface

LSST sims interface to gcr-catalogs
BSD 3-Clause "New" or "Revised" License
1 stars 5 forks source link

U/danielsf/separate/objects #52

Closed danielsf closed 6 years ago

danielsf commented 6 years ago

This PR expands #49 so that, when writing PhoSim InstanceCatalogs, disks, bulges, and AGN/SNe all get written to separate gzipped txt files which are passed to PhoSim through includeobj. This is to make life easier on the generic catalog reader when it tries to load InstanceCatalogs.

This PR also supersedes #47. I was touching a lot of the same lines, so I just tried to fold in Jim's work removing the customized ICRS catalog classes.

danielsf commented 6 years ago

PR #49 was held up because I was not sure that it would handle sprinkled SNe correctly. I have since tested this code and it does handle them as it should.

We will still need the following Twinkles PR

https://github.com/LSSTDESC/Twinkles/pull/472

in order for this PR to work

cwwalter commented 6 years ago

Seperating components sounds great. But, I wonder if it is a good idea to seperate bulge and disks. Theses are really the same object and it will make dealing with theinstance catalogs and doing manual checks or manually pulling a few galaxies out of the instance catalogs with standard Unix tools for examination or simulation tedious and error prone.

danielsf commented 6 years ago

I can go either way on the "do we separate bulges and disks" question. I just noticed that, in Javier's InstanceCatalog reader, he included code to inspect the uniqueId to figure out which components were bulges and which were disks. One argument in favor of separating the components: Javier finds bulges and disks by looking at uniqueId%1024 and comparing to the offset applied to uniqueId for each component to figure out which are bulges and which are disks. This offset is somewhat arbitrary and will almost certainly change in cosmoDC2 when we stop using the protoDC2 database emulator classes. Conversely, if the components were separate and you needed to figure out which bulges and disks to compare, you know that you are always looking for objects with identical uniqueId//1024 (i.e. the 1024 is less likely to change than the offset).

yymao commented 6 years ago

It certainly would make the reader a bit easier to write if bulges and disks are in separate files. But the particular part of the code @danielsf mentioned above can be configurable (i.e. we can specify in the config file the corresponding between uniqueId%1024 and disk/bulge). Hence, either way would still work.

danielsf commented 6 years ago

@jbkalmbach Just want to make sure you saw this, since I issued it while you were at the T/VS working group meeting

jbkalmbach commented 6 years ago

Oh, sorry I did miss this. I will take a look in the morning.

danielsf commented 6 years ago

My understanding was that the truth catalogs were meant to be a way to explicitly separate out sprinkled objects from unsprinkled objects. Truth information for unsprinkled objects can be harvested from the extragalactic catalog after image generation, but, since the sprinkler will replace some cosmoDC2 galaxies with OM10 galaxies, we needed a way to know which objects had been replaced by the sprinkler. I may be misunderstanding something, though.

danielsf commented 6 years ago

I no one objects, I will merge this at noon today.

cwwalter commented 6 years ago

So, just to be clear, going forward after this bulges and disks will be in their own object.gz files. Is that correct?

danielsf commented 6 years ago

That is correct.

danielsf commented 6 years ago

I wasn't sure where that conversation was, though. It will be easy to put bulges and disks back together if we decide that is better.

danielsf commented 6 years ago

@jbkalmbach am I good to merge the supporting PR on the Sprinkler?

jbkalmbach commented 6 years ago

Yeah, I don't have any objections. I was just trying to get more information on what each output is going to be used for and maybe that discussion is already/should be somewhere in the DC2 repo.