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-24517: Reading of raws from existing imsim & phosim repos broken by DM-23584 #203
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok.
.gitignore
Outdated
@@ -17,4 +17,6 @@ ts8/CALIB/calibRegistry.sqlite3 | |||
ts8/CALIB/qe_curve/ | |||
lsstcam/CALIB/calibRegistry.sqlite3 | |||
lsstcam/CALIB/qe_curve/ | |||
|
|||
data/input/phosim/calibRegistry.sqlite3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't understand why these files appear so I worry that ignoring them makes it easy to forget there is a possible problem somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it didn't seem like they should be there to me either, but I didn't dig deep enough to figure out where they're coming from. I could make a ticket for someone who knows better to look into it. Shall I leave this in to avoid the annoyance for now but update the commit message (or comment in the .gitignore
file directly?) to indicate they should be removed with that ticket...or should I just get rid of it altogether?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally did what I should have done ages ago and make the data directory read only. The new files are created because of a bug in the gen2to3 code. Please don't merge the .gitignore change. I'll make a new ticket for the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I'll remove that commit and restart Jenkins to be sure...and it looks like the current one had an unrelated failure on centos-6 (osx passed and centos-7 made it past the failure point)? https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/31688/pipeline/48
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That ip_isr error is weird. I wouldn't insist on doing a whole new jenkins run for a .gitignore change since .gitinore is not anything that's tested during the build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well...I'm overly paranoid, so it's already going (and can't hurt 😉). Thanks again!
Prior to DM-23584, the templates in obs_lsst/policy/lsstCamMapper.yaml were compatible with imsim conventions. However, the change in the expId length from 8 to 13 digits in DM-23584 rendered them incompatible with the imsim standards (which remains "08d" for the expId field). This adds the policy file policy/imsim/imsimMapper.yaml with the appropriate overrides and updates python/lsst/obs/lsst/imsim.py to apply them.
The template length for expId is 8 digits for imsim, so the test and test data should reflect this.
The phosim templates match those of imsim, so here we load the latter overrides to the lsstCamMapper.yaml templates to those of phosim to keep the two sims in sync.
This is equivalent to what was done for imsim on DM-21241 on this commit: 26d62a0 The only difference is that there are two possible header keywords containing the rotation angle ("ROTANGZ" or "ROTANGLE") so both are checked for in the header.
23f9aa0
to
3d3b73f
Compare
No description provided.