Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run 2.1 preflight checklist #188

Closed
13 tasks done
cwwalter opened this issue Nov 19, 2018 · 14 comments
Closed
13 tasks done

Run 2.1 preflight checklist #188

cwwalter opened this issue Nov 19, 2018 · 14 comments

Comments

@cwwalter
Copy link
Member

cwwalter commented Nov 19, 2018

Here is a master list of issues to address before we can start 2.1i:

Please check these off when we know we have stopped performance optimization for the 2.1 run time scale.

Test for correctness (these tests might be tracked elsewhere; no issues yet)

Note: to see a good overview the issues which are grouped in columns, look at this project:

https://github.com/LSSTDESC/imSim/projects/1

Keep up-to-date and note changes in comments below.

@cwwalter cwwalter added the DC2 label Nov 19, 2018
@cwwalter
Copy link
Member Author

I accidentally left out issue #178 -Determining the list of headers to add to the primary HDU. Now it is in the list.

@jchiang87
Copy link
Collaborator

jchiang87 commented Nov 21, 2018

Of the items listed above, I think only a few are strictly needed to proceed with Run2.1i:

The first one listed here entails a release tag of GalSim at v2.1 and a corresponding lsst_sims release. The third one would be implemented in obs_lsst. Among the other items in the original list, the sims_skybrightness would be handled by using lsst_sims 2_10_0 or later (we're at 2_11_1 now), and an open-ended item like #187 doesn't really belong on a prerequisite list like this.

@cwwalter
Copy link
Member Author

cwwalter commented Nov 21, 2018

Scott has confirmed that the sky brightness fix is in 2_11

(oops crossed with Jim above)

@cwwalter
Copy link
Member Author

cwwalter commented Nov 21, 2018

Well, I agree the first three in the list are the ones that are absolutely necessary to be able to run the program, but I think in particular skipping:

would be very problematic. Those both include things that we have to or should add at run time that we need later to use the files for testing, processing and analysis.

I agree the e-image fix is not necessary but I think would be useful for early running based on our experiences so far trying to process things through DM.

I put the performance issue in since, in the discussion, I thought there were a few things that Mike still wanted to check and I wanted it there as a reminder to make sure we either have signed off on it being done for now, or knew there would be more changes down the line.

@cwwalter
Copy link
Member Author

I moved the performance check into a separate sub-list following Jim's suggestion.

@cwwalter
Copy link
Member Author

In that new category I also added partially implementing binary instance catalog format as a possibility as Jim suggested at the last imSim video meeting. There are more details in #71.

@jchiang87
Copy link
Collaborator

Extend centroid file with object class, entries for skipped object and realized flux (#185)
 Add necessary headers to HDU (#178)

I agree these would be nice to have, but I don't see how they are strictly necessary. We already know that the current headers allow the data to be processed through the coadd and forced photometry of the drp processing, and we won't even have centroid files for real data. #178 is also a moving target--so we can add what's requested now (even though we know those items aren't needed for processing), but until we have a complete list, that issue won't be closed before (assuming it ever is) we would want to have Run2.1i data in hand.

@cwwalter
Copy link
Member Author

Re: discussion of #185 and #178:

  • Determine list of image metadata keywords that should be in the primary HDU #178: I agree that we won't know the full list. So we can't wait to get the entire list before we begin. However, there are a few things we know we should add, imSim version numbers and random # seeds, the headers Tim just listed, and the environmental variables listed needed for jointcal/DC etc. So, actually what I mean by including that in the list is that we should come up with the list of what we need to include before we start 2.1. I edited the top post to try to make that clearer now. Thanks. Perhaps we should make an entry on the top of that issue where we start to list things that we should include before we start? So, to be clear: let's just add the minimum number of things we know about know, not wait until we have an exhaustive list.

  • Extend centroid file with information for faint objects #185: I think this is important for as soon as immediate testing/validation of how the dimmer objects that went through the new code are being handled. We are adding new classes of objects with simplified SEDs etc. These files will probably last us a few years and people are going to be doing careful studies with them. We need to be able to tag these objects during the studies (and see all of the objects that should have actually been there) to understand if any differences or problems that are noticed are due to these approximations etc. I think this could be particularly important for the WL and PZ groups. Based on my experience really intensively using simulation output and experiencing how people people try to figure out and resolve issues they come across, I think this is going to be very important. We can't do this after the fact. The decision on the path taken for drawing the image is done at run time and depends on what other objects are near it during rendering. The production is such an intensive operation I think we should do this now.

@rmandelb
Copy link

@cwwalter - my impression had been that the SED simplification would only be used for galaxies faint enough that they aren't part of a canonical WL/LSS sample, and so we wouldn't care about their photo-z (only what they do to the photo-z of others nearby, which should be minimally affected by the SED simplification). If somebody can remind me what flux criterion we're using in each band, I can confirm this given a typical magnitude or SNR cut and typical galaxy colors.

@rmjarvis
Copy link
Contributor

It's <100 photons for single epoch, which I believe converts to m > 26. (Assuming zeropoint = 31)

@rmjarvis
Copy link
Contributor

But also, I agree with Chris that this would be vv good to have this information in the output truth catalog regardless of whether we are using these objects for WL or PZ.

@cwwalter
Copy link
Member Author

Removing partially implementing binary instance format for this list now due to time constraints. We will work on this next year.

@cwwalter
Copy link
Member Author

cwwalter commented Nov 27, 2018

#130 is now closed but lsst/obs_lsst#41 should be merged into the new obs_lsst package.

@cwwalter
Copy link
Member Author

cwwalter commented Dec 3, 2018

lsst/obs_lsst#41 has now been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants