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

DM-17530: Add support for BNL TS3 data #70

Merged
merged 7 commits into from Feb 13, 2019
Merged

DM-17530: Add support for BNL TS3 data #70

merged 7 commits into from Feb 13, 2019

Conversation

timj
Copy link
Member

@timj timj commented Jan 29, 2019

  • Basic translator so far.
  • Need to add mappers and camera definition files.

@mfisherlevine should we use the #66 approach of having R00 be E2V data and R01 be ITL data? Do you have a list of all the detector serial numbers to be supported? Do we have a raft per sensor to make the camera yaml easier?

Each detector is assigned a single raft. 435 detectors are defined
includeing ITL and E2V.  The amplifier definitions are a clone
of TS8.
@timj
Copy link
Member Author

timj commented Feb 8, 2019

@mfisherlevine take a look at this and see what you think. I can ingest those two TS3 files now without a problem. I need to add nulled out versions to the local obs_lsst butler repo so I can run the tests.

Copy link
Contributor

@mfisherlevine mfisherlevine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prize for the world's most samey PR? Regardless, looks good, and many thanks.

python/lsst/obs/lsst/ts3.py Show resolved Hide resolved
python/lsst/obs/lsst/translators/ts8.py Outdated Show resolved Hide resolved
python/lsst/obs/lsst/translators/ts3.py Show resolved Hide resolved
python/lsst/obs/lsst/translators/lsstsim.py Outdated Show resolved Hide resolved
@mfisherlevine
Copy link
Contributor

OK, looks good. Thanks Tim!

_DETECTOR_NAME = "S00"


class LsstTS3Translator(StubTranslator):
class LsstTS3Translator(LsstSimTranslator):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, that's an interesting change. I'm sure it makes sense, but important from the Sims translator rather than something that sounds like some sort of base class is surprising.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's telling you that imsim and phosim share infrastructure that is also now needed by TS3 (and TS8 it turns out) and that at this point the parent class name is wrong.

tests/test_ts3.py Outdated Show resolved Hide resolved
exptimes = {'raw': 30.611}
detectorIds = {'raw': 71}
detector_names = {'raw': 'R071_S00'}
detector_serials = {'raw': 'ITL-3800C-098'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice choice of chips. Is that a coincidence, or was that deliberate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You chose them (this is zeroed out data from the testdata repository).

Also document how the TS3 data was created.
@timj timj merged commit 84ca5e5 into master Feb 13, 2019
@timj timj deleted the tickets/DM-17530 branch May 12, 2022 16:46
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

Successfully merging this pull request may close these issues.

None yet

2 participants