Updates to use visitInfo Tickets/dm 6625 #17
Conversation
parejkoj
force-pushed
the
tickets/DM-6625
branch
from
2b800ed
to
778dcad
Compare
Use detector ID to identify CCD, instead of survey-specific "ccd" Get filter from calexp instead of dataRef
parejkoj
force-pushed
the
tickets/DM-6625
branch
from
e3a9be0
to
5149ff9
Compare
| pdb.set_trace() | ||
| # plt.show() | ||
| # import pdb | ||
| # pdb.set_trace() |
There was a problem hiding this comment.
Remove the commented-out debugging code.
There was a problem hiding this comment.
Sure. All the plotting code is going to be pulled into another place soon.
| """Make quiver plots of the WCS deltas for each CCD in each visit.""" | ||
|
|
||
| for cat in visitCatalogs: | ||
| fig = plt.figure() | ||
| fig.set_tight_layout(True) | ||
| # fig.set_tight_layout(True) |
| }; | ||
|
|
||
| #endif /* ASSOCIATIONS__H */ | ||
|
|
||
| }} // end of namespaces | ||
| } | ||
| } // end of namespaces |
There was a problem hiding this comment.
Looks like just changes in indentation, which look fine.
|
|
||
| }; | ||
|
|
||
|
|
||
| }} | ||
| } | ||
| } | ||
| #endif /* ASTROMFIT__H */ |
There was a problem hiding this comment.
The new variable names are much more intuitive, thank you for updating them.
There was a problem hiding this comment.
I was having enough trouble tracking one instance through multiple files...
| //! | ||
| Point ParallacticVector() const {return Point(tgz*coseta, tgz*sineta);} | ||
| //! | ||
| Point RefractionVector() const {return Point(tgz * coseta, tgz * sineta);} |
There was a problem hiding this comment.
I thought our coding standards specified no space around multiplication operators.
There was a problem hiding this comment.
Oof. I've updated my SublimeAStyleFormatter config (that's what did the cleanup) to not put spaces around operators. I'll see if I can easily get these sorted out manually.
| # # first run of jointcal on this data. We should always do better than | ||
| # # this in the future! | ||
| # relative_error = 10e-3*u.arcsecond | ||
| # self._testJointCalTask(6, relative_error, absolute_error) |
There was a problem hiding this comment.
Remove this commented-out test (and line 52)?
There was a problem hiding this comment.
This is in flux right now because the hsc data set changed recently. I'm going to leave it for when I sort out how to run on the new hsc data (I don't remember which set of visits goes with which).
| from __future__ import division, absolute_import, print_function | ||
|
|
||
| import matplotlib | ||
| # Have to import matplotlib at the top level, because it can be imported by |
There was a problem hiding this comment.
Now the previous matplotlib import I saw makes sense.
| import unittest | ||
| import os | ||
|
|
||
| from astropy import units as u |
There was a problem hiding this comment.
Could you just 'from astropy import units'? I find 'u' short and confusing.
There was a problem hiding this comment.
That's the shorthand that astropy recommends: http://docs.astropy.org/en/stable/units/#getting-started
| def setUp(self): | ||
| jointcalTestBase.JointcalTestBase.setUp(self) | ||
| self.do_plot = do_plot | ||
| self.match_radius = 0.1*lsst.afw.geom.arcseconds |
There was a problem hiding this comment.
Why are you using astropy arcseconds above, and lsst.afw.geom.arcseconds here?
There was a problem hiding this comment.
afw.arcseconds doesn't behave well with matplotlib, but astropy does, whereas lsst needs the afw version for e.g. IcrsCoord constructors.. It's very unfortunate, but I don't think there's a way around it until we can e.g. construct afw objects with astropy units or have afw things behave better with matplotlib.
There was a problem hiding this comment.
Also: the way afw angles were being managed made it difficult for me to perform some of the calculations, but it was much easier to manage with astropy.
| self._prep_reference_loader(center, radius) | ||
|
|
||
| self.input_dir = os.path.join(data_dir, 'twinkles1') | ||
| self.all_visits = range(840, 850) |
There was a problem hiding this comment.
If you hard-code in numbers, please explain them in a comment.
There was a problem hiding this comment.
Those are the visits in the validation data set? They'll always be hard coded in (you always have to specify the visits for processing), and all_visits is documented in jointcalTestBase.py
parejkoj
force-pushed
the
tickets/DM-6625
branch
from
5149ff9
to
9ac4287
Compare
Fixed camelCase and expanded some names in AstromFit and associated classes. expTime isn't used anywhere, so why are we saving it? Fix / and * operator spacing
Removed old star selector code: don't need it any more. Remove old lsstSim hack to test on a fake catalog with run() Remove unneeded imports from test_jointcal_lsstSim
parejkoj
force-pushed
the
tickets/DM-6625
branch
from
9ac4287
to
2d8945e
Compare
Now that DM-5503 is done, it's time to get lsstSim to work without my FakeRef hack (and properly deal with all camera metadata) by using VisitInfo. This PR encompasses all of the work related to that.