LSSTDESC / imSim

GalSim based Rubin Observatory image simulation package
https://lsstdesc.org/imSim
BSD 3-Clause "New" or "Revised" License
36 stars 15 forks source link

Overriding eval_variables.sband in the YAML file does not override all band variables. #430

Closed cwwalter closed 10 months ago

cwwalter commented 10 months ago

In testing that Jim and I are doing I was overriding the eval variable eval_variables.sband for a yaml file which had an entry like this:

input.sky_catalog:
  file_name: /sdf/data/rubin/user/jchiang/imSim/skyCatalogs_v2/skyCatalog.yaml
  approx_nobjects: 120000
  band: { type: OpsimData, field: band }
  mjd: { type: OpsimData, field: mjd }

# Get the metadata information from here.
input.opsim_data.file_name: /sdf/data/rubin/user/jchiang/imSim/multiproc_tests/phosim_cat_182850.txt

(which has filter=3 corresponding to i-band) by doing

eval_variables.sband=u

on the command line. The resulting run took quite a long time and had very bright objects. When Jim examned the headers int he amp files he found that physical filter was "i_39" which is i-band.

So, is seems that the eval variables are not being used everywhere. This will be relevant in AOS simulations.

jchiang87 commented 10 months ago

I'll do a more definitive check on what band the code is actually using when it asks for the object flux.

jchiang87 commented 10 months ago

I've confirmed that this command line override eval_variables.sband=u does not get propagated to the SkyCatalogInterface class. Something needs to make sure that SkyCatalogLoader gets the new band value, since that class is the one that interfaces to the config parameters.

rmjarvis commented 10 months ago

Right. The imsim-config-skycat.yaml file should use the eval_variables version.

input.sky_catalog:
    file_name: default_sky_cat.yaml
    band: $band
    mjd: { type: OpsimData, field: mjd }

Right now it is always reading from OpsimData regardless of what everyone else is using for their band values.

cwwalter commented 10 months ago

Ah. right. Thanks! I'll try testing with that.

cwwalter commented 10 months ago

OK, I can confirm this works.

The first test I did, it didn't work since it turns out my yaml file had also copied part of that block before we changed everything to use $band.

Interestingly, when that band is wrong, it takes about 10 hours to finish. I didn't really spend time trying to figure out why, but I suspect it is something like the FFT threshold gets calculated incorrectly in that case.

I've been wondering if we could do something like use

@image.bandpass instead of $band in these cases since we set

bandpass: { type: RubinBandpass, band: "$band" }

in the main file. This would make it so they couldn't get out-of-sync. Am I missing anything with that Mike?

rmjarvis commented 10 months ago

@image.bandpass is the constructed galsim.Bandpass object. If something can handle that in lieu of a str, then it's fine to use it. But I think most places we use a band item, it expects a string. If instead you use @image.bandpass.band, then things will break if the user decides to use a different bandpass, which might not have a band item in the dict. (Might not be a dict at all for that matter if they run from python and just give an already constructed Bandpass there.)

cwwalter commented 10 months ago

Ok thanks. I'll close this and push the $band fix to the same brach with the other fixes for this set of issues I found.