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.

Sign up for GitHub

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

Jump to bottom

DM-13944: add id to VisitInfo #595

Merged
merged 3 commits into from
Jun 18, 2021
Merged

DM-13944: add id to VisitInfo #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d131c6
Add id field to VisitInfo
parejkoj Jun 11, 2021
b21e12b
Add visitInfo readonly properties
parejkoj Jun 15, 2021
fe74e73
Add pybind11 properties to Exposure
parejkoj Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions include/lsst/afw/image/VisitInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,15 @@ class VisitInfo : public table::io::PersistableFacade<VisitInfo>, public typehan
* @param[in] observatory observatory longitude, latitude and altitude
* @param[in] weather basic weather information for computing air mass
* @param[in] instrumentLabel The short name of the instrument that took this data (e.g. "HSC")
* @param[in] id The identifier of this full focal plane data.
*/
explicit VisitInfo(table::RecordId exposureId, double exposureTime, double darkTime,
daf::base::DateTime const &date, double ut1, lsst::geom::Angle const &era,
lsst::geom::SpherePoint const &boresightRaDec,
lsst::geom::SpherePoint const &boresightAzAlt, double boresightAirmass,
lsst::geom::Angle const &boresightRotAngle, RotType const &rotType,
coord::Observatory const &observatory, coord::Weather const &weather,
std::string const &instrumentLabel)
std::string const &instrumentLabel, table::RecordId const &id)
Copy link
Member

Choose a reason for hiding this comment

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

From butler this is a python integer (64-bit I think in registry) so I'm not entirely why this is a RecordId. Does that mean something special such that you could use it as a primary key in an afw table or something?

Copy link
Member

Choose a reason for hiding this comment

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

RecordId is a typedef to a signed 64-bit int. Don't know if it's better or worse to use the typedef here; it guarantees consistency with the ID columns in afw.table that sometimes hold these values, but it's also not a desirable dependency (even if it's not the only occurrence of this dependency).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using the same type as exposureId, since this effectively replaces that. table::RecordId is int64.

: _exposureId(exposureId),
_exposureTime(exposureTime),
_darkTime(darkTime),
Expand All @@ -107,7 +108,8 @@ class VisitInfo : public table::io::PersistableFacade<VisitInfo>, public typehan
_rotType(rotType),
_observatory(observatory),
_weather(weather),
_instrumentLabel(instrumentLabel){};
_instrumentLabel(instrumentLabel),
_id(id){};

explicit VisitInfo(daf::base::PropertySet const &metadata);

Expand Down Expand Up @@ -181,6 +183,8 @@ class VisitInfo : public table::io::PersistableFacade<VisitInfo>, public typehan

std::string getInstrumentLabel() const { return _instrumentLabel; }

table::RecordId getId() const { return _id; }

/**
* Get parallactic angle at the boresight
*
Expand Down Expand Up @@ -225,6 +229,7 @@ class VisitInfo : public table::io::PersistableFacade<VisitInfo>, public typehan
coord::Observatory _observatory;
coord::Weather _weather;
std::string _instrumentLabel;
table::RecordId _id;
};

std::ostream &operator<<(std::ostream &os, VisitInfo const &visitInfo);
Expand Down
25 changes: 23 additions & 2 deletions python/lsst/afw/image/_visitInfo.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,15 @@ void declareVisitInfo(lsst::utils::python::WrapperCollection &wrappers) {
lsst::geom::Angle const &, lsst::geom::SpherePoint const &,
lsst::geom::SpherePoint const &, double, lsst::geom::Angle const &,
RotType const &, coord::Observatory const &, coord::Weather const &,
std::string const &>(),
std::string const &, table::RecordId const &>(),
"exposureId"_a = 0, "exposureTime"_a = nan, "darkTime"_a = nan,
"date"_a = daf::base::DateTime(), "ut1"_a = nan, "era"_a = nanAngle,
"boresightRaDec"_a = lsst::geom::SpherePoint(nanAngle, nanAngle),
"boresightAzAlt"_a = lsst::geom::SpherePoint(nanAngle, nanAngle),
"boresightAirmass"_a = nan, "boresightRotAngle"_a = nanAngle,
"rotType"_a = RotType::UNKNOWN,
"observatory"_a = coord::Observatory(nanAngle, nanAngle, nan),
"weather"_a = coord::Weather(nan, nan, nan), "instrumentLabel"_a = "");
"weather"_a = coord::Weather(nan, nan, nan), "instrumentLabel"_a = "", "id"_a = 0);
cls.def(py::init<daf::base::PropertySet const &>(), "metadata"_a);
cls.def(py::init<VisitInfo const &>(), "visitInfo"_a);

Expand Down Expand Up @@ -105,6 +105,27 @@ void declareVisitInfo(lsst::utils::python::WrapperCollection &wrappers) {
cls.def("getLocalEra", &VisitInfo::getLocalEra);
cls.def("getBoresightHourAngle", &VisitInfo::getBoresightHourAngle);
cls.def("getInstrumentLabel", &VisitInfo::getInstrumentLabel);
cls.def("getId", &VisitInfo::getId);

/* readonly property accessors */
cls.def_property_readonly("exposureTime", &VisitInfo::getExposureTime);
cls.def_property_readonly("darkTime", &VisitInfo::getDarkTime);
cls.def_property_readonly("date", &VisitInfo::getDate);
cls.def_property_readonly("ut1", &VisitInfo::getUt1);
cls.def_property_readonly("era", &VisitInfo::getEra);
cls.def_property_readonly("boresightRaDec", &VisitInfo::getBoresightRaDec);
cls.def_property_readonly("boresightAzAlt", &VisitInfo::getBoresightAzAlt);
cls.def_property_readonly("boresightAirmass", &VisitInfo::getBoresightAirmass);
cls.def_property_readonly("boresightParAngle", &VisitInfo::getBoresightParAngle);
cls.def_property_readonly("boresightRotAngle", &VisitInfo::getBoresightRotAngle);
cls.def_property_readonly("rotType", &VisitInfo::getRotType);
cls.def_property_readonly("observatory", &VisitInfo::getObservatory);
cls.def_property_readonly("weather", &VisitInfo::getWeather);
cls.def_property_readonly("isPersistable", &VisitInfo::isPersistable);
cls.def_property_readonly("localEra", &VisitInfo::getLocalEra);
cls.def_property_readonly("boresightHourAngle", &VisitInfo::getBoresightHourAngle);
cls.def_property_readonly("instrumentLabel", &VisitInfo::getInstrumentLabel);
cls.def_property_readonly("id", &VisitInfo::getId);

utils::python::addOutputOp(cls, "__str__");
});
Expand Down
16 changes: 16 additions & 0 deletions python/lsst/afw/image/exposure/_exposure.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,30 +98,46 @@ PyExposure<PixelT> declareExposure(lsst::utils::python::WrapperCollection &wrapp
cls.def("setMetadata", &ExposureT::setMetadata, "metadata"_a);
cls.def("getWidth", &ExposureT::getWidth);
cls.def("getHeight", &ExposureT::getHeight);
cls.def_property_readonly("width", &ExposureT::getWidth);
cls.def_property_readonly("height", &ExposureT::getHeight);
cls.def("getDimensions", &ExposureT::getDimensions);
cls.def("getX0", &ExposureT::getX0);
cls.def("getY0", &ExposureT::getY0);
cls.def_property_readonly("x0", &ExposureT::getX0);
cls.def_property_readonly("y0", &ExposureT::getY0);
cls.def("getXY0", &ExposureT::getXY0);
cls.def("setXY0", &ExposureT::setXY0, "xy0"_a);
cls.def("getBBox", &ExposureT::getBBox, "origin"_a = PARENT);
cls.def("getWcs", (std::shared_ptr<geom::SkyWcs>(ExposureT::*)()) & ExposureT::getWcs);
cls.def_property_readonly(
"wcs", (std::shared_ptr<geom::SkyWcs>(ExposureT::*)()) & ExposureT::getWcs);
cls.def("setWcs", &ExposureT::setWcs, "wcs"_a);
cls.def("hasWcs", &ExposureT::hasWcs);
cls.def("getDetector", &ExposureT::getDetector);
cls.def_property_readonly("detector", &ExposureT::getDetector);
cls.def("setDetector", &ExposureT::setDetector, "detector"_a);
cls.def("getFilter", &ExposureT::getFilter);
cls.def("setFilter", &ExposureT::setFilter, "filter"_a);
cls.def("getFilterLabel", &ExposureT::getFilterLabel);
cls.def_property_readonly("filterLabel", &ExposureT::getFilterLabel);
cls.def("setFilterLabel", &ExposureT::setFilterLabel, "filterLabel"_a);

cls.def("getPhotoCalib", &ExposureT::getPhotoCalib);
cls.def_property_readonly("photoCalib", &ExposureT::getPhotoCalib);
cls.def("setPhotoCalib", &ExposureT::setPhotoCalib, "photoCalib"_a);
cls.def("getPsf", (std::shared_ptr<detection::Psf>(ExposureT::*)()) & ExposureT::getPsf);
cls.def_property_readonly(
"psf", (std::shared_ptr<detection::Psf>(ExposureT::*)()) & ExposureT::getPsf);
cls.def("setPsf", &ExposureT::setPsf, "psf"_a);
cls.def("hasPsf", &ExposureT::hasPsf);
cls.def("getInfo", (std::shared_ptr<ExposureInfo>(ExposureT::*)()) & ExposureT::getInfo);
cls.def_property_readonly(
"info", (std::shared_ptr<ExposureInfo>(ExposureT::*)()) & ExposureT::getInfo);
cls.def("setInfo", &ExposureT::setInfo, "exposureInfo"_a);

cls.def_property_readonly("visitInfo",
[](ExposureT &self) { return self.getInfo()->getVisitInfo(); });

cls.def("subset", &ExposureT::subset, "bbox"_a, "origin"_a = PARENT);

cls.def("writeFits", (void (ExposureT::*)(std::string const &) const) & ExposureT::writeFits);
Expand Down
33 changes: 25 additions & 8 deletions src/image/VisitInfo.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ namespace {
// Version history:
// unversioned: original VisitInfo schema
// 1: file versioning, instrument label
int constexpr SERIALIZATION_VERSION = 1;
// 2: id field
int constexpr SERIALIZATION_VERSION = 2;

auto const nan = std::numeric_limits<double>::quiet_NaN();

Expand Down Expand Up @@ -213,6 +214,8 @@ class VisitInfoSchema {
table::Key<std::string> instrumentLabel;
table::Key<int> version;

table::Key<table::RecordId> idnum;

static std::string const VERSION_KEY;

static VisitInfoSchema const& get() {
Expand Down Expand Up @@ -270,7 +273,10 @@ class VisitInfoSchema {
"instrumentlabel", "Short name of the instrument that took this data", "", 0)),

// for internal support
version(schema.addField<int>(VERSION_KEY, "version of this VisitInfo")) {}
version(schema.addField<int>(VERSION_KEY, "version of this VisitInfo")),

idnum(schema.addField<table::RecordId>("idnum", "identifier of this full focal plane exposure",
"")) {}
};
std::string const VisitInfoSchema::VERSION_KEY = "version";

Expand All @@ -290,6 +296,7 @@ class VisitInfoFactory : public table::io::PersistableFactory {

// Version-dependent fields
std::string instrumentLabel = version >= 1 ? record.get(keys.instrumentLabel) : "";
table::RecordId id = version >= 2 ? record.get(keys.idnum) : 0;

std::shared_ptr<VisitInfo> result(
new VisitInfo(record.get(keys.exposureId), record.get(keys.exposureTime),
Expand All @@ -303,7 +310,7 @@ class VisitInfoFactory : public table::io::PersistableFactory {
record.get(keys.elevation)),
coord::Weather(record.get(keys.airTemperature), record.get(keys.airPressure),
record.get(keys.humidity)),
instrumentLabel));
instrumentLabel, id));
return result;
}

Expand Down Expand Up @@ -337,7 +344,7 @@ int stripVisitInfoKeywords(daf::base::PropertySet& metadata) {
"TIME-MID", "MJD-AVG-UT1", "AVG-ERA", "BORE-RA", "BORE-DEC",
"BORE-AZ", "BORE-ALT", "BORE-AIRMASS", "BORE-ROTANG", "ROTTYPE",
"OBS-LONG", "OBS-LAT", "OBS-ELEV", "AIRTEMP", "AIRPRESS",
"HUMIDITY", "INSTRUMENT"};
"HUMIDITY", "INSTRUMENT", "IDNUM"};
for (auto&& key : keyList) {
if (metadata.exists(key)) {
metadata.remove(key);
Expand Down Expand Up @@ -379,6 +386,9 @@ void setVisitInfoMetadata(daf::base::PropertyList& metadata, VisitInfo const& vi
setDouble(metadata, "HUMIDITY", weather.getHumidity(), "Relative humidity (%)");
setString(metadata, "INSTRUMENT", visitInfo.getInstrumentLabel(),
"Short name of the instrument that took this data");
if (visitInfo.getId() != 0) {
metadata.set("IDNUM", visitInfo.getId(), "identifier of this full focal plane exposure");
}
}

} // namespace detail
Expand All @@ -401,11 +411,16 @@ VisitInfo::VisitInfo(daf::base::PropertySet const& metadata)
getDouble(metadata, "OBS-ELEV")),
_weather(getDouble(metadata, "AIRTEMP"), getDouble(metadata, "AIRPRESS"),
getDouble(metadata, "HUMIDITY")),
_instrumentLabel(getString(metadata, "INSTRUMENT")) {
_instrumentLabel(getString(metadata, "INSTRUMENT")),
_id(0) {
auto key = "EXPID";
if (metadata.exists(key)) {
_exposureId = metadata.getAsInt64(key);
}
key = "IDNUM";
if (metadata.exists(key)) {
_id = metadata.getAsInt64(key);
}

key = "EXPTIME";
if (metadata.exists(key)) {
Expand Down Expand Up @@ -458,14 +473,14 @@ bool VisitInfo::operator==(VisitInfo const& other) const {
_boresightAzAlt == other.getBoresightAzAlt() && _boresightAirmass == other.getBoresightAirmass() &&
_boresightRotAngle == other.getBoresightRotAngle() && _rotType == other.getRotType() &&
_observatory == other.getObservatory() && _weather == other.getWeather() &&
_instrumentLabel == other.getInstrumentLabel();
_instrumentLabel == other.getInstrumentLabel() && _id == other.getId();
}

std::size_t VisitInfo::hash_value() const noexcept {
// Completely arbitrary seed
return utils::hashCombine(17, _exposureId, _exposureTime, _darkTime, _date, _ut1, _era, _boresightRaDec,
_boresightAzAlt, _boresightAirmass, _boresightRotAngle, _rotType, _observatory,
_weather, _instrumentLabel);
_weather, _instrumentLabel, _id);
}

std::string VisitInfo::getPersistenceName() const { return getVisitInfoPersistenceName(); }
Expand Down Expand Up @@ -497,6 +512,7 @@ void VisitInfo::write(OutputArchiveHandle& handle) const {
record->set(keys.humidity, weather.getHumidity());
record->set(keys.instrumentLabel, getInstrumentLabel());
record->set(keys.version, SERIALIZATION_VERSION);
record->set(keys.idnum, getId());
handle.saveCatalog(cat);
}

Expand Down Expand Up @@ -542,7 +558,8 @@ std::string VisitInfo::toString() const {
buffer << "rotType=" << static_cast<int>(getRotType()) << ", ";
buffer << "observatory=" << getObservatory() << ", ";
buffer << "weather=" << getWeather() << ", ";
buffer << "instrumentLabel=" << getInstrumentLabel();
buffer << "instrumentLabel='" << getInstrumentLabel() << "', ";
buffer << "id=" << getId();
buffer << ")";
return buffer.str();
}
Expand Down
4 changes: 4 additions & 0 deletions tests/data/visitInfo-version-2.fits
View file
Open in desktop

Large diffs are not rendered by default.

21 changes: 21 additions & 0 deletions tests/test_exposure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ def testGetMaskedImage(self):
# Check Exposure.getWidth() returns the MaskedImage's width
self.assertEqual(crOnlyWidth, self.exposureCrOnly.getWidth())
self.assertEqual(crOnlyHeight, self.exposureCrOnly.getHeight())
# check width/height properties
self.assertEqual(crOnlyWidth, self.exposureCrOnly.width)
self.assertEqual(crOnlyHeight, self.exposureCrOnly.height)

def testProperties(self):
self.assertMaskedImagesEqual(self.exposureMiOnly.maskedImage,
Expand Down Expand Up @@ -181,6 +184,9 @@ def testProperties(self):
self.exposureMiOnly.variance = var3
self.assertImagesEqual(self.exposureMiOnly.variance, var3)

# Test the property getter for a null VisitInfo.
self.assertIsNone(self.exposureMiOnly.visitInfo)

def testGetWcs(self):
"""Test that a WCS can be obtained from each Exposure created with
a WCS, and that an Exposure lacking a WCS returns None.
Expand Down Expand Up @@ -217,6 +223,8 @@ def testExposureInfoConstructor(self):
self.assertEqual(exposure.getFilterLabel(), gFilterLabel)

self.assertTrue(exposure.getInfo().hasWcs())
# check the ExposureInfo property
self.assertTrue(exposure.info.hasWcs())
self.assertEqual(exposure.getInfo().getWcs().getPixelOrigin(),
self.wcs.getPixelOrigin())
self.assertEqual(exposure.getInfo().getDetector().getName(),
Expand Down Expand Up @@ -274,6 +282,11 @@ def testSetExposureInfo(self):
self.assertEqual(exposure.getFilter(), gFilter)
self.assertEqual(exposure.getFilterLabel(), gFilterLabel)

# test properties
self.assertEqual(exposure.detector.getName(), self.detector.getName())
self.assertEqual(exposure.filterLabel, gFilterLabel)
self.assertEqual(exposure.wcs, self.wcs)

def testDefaultFilter(self):
"""Test that old convention of having a "default" filter replaced with `None`.
"""
Expand Down Expand Up @@ -315,6 +328,12 @@ def testVisitInfoFitsPersistence(self):
self.assertEqual(rtExposure.getFilter(), gFilter)
self.assertEqual(rtExposure.getFilterLabel(), gFilterLabel)

# Test property getters.
self.assertEqual(rtExposure.photoCalib, photoCalib)
self.assertEqual(rtExposure.filterLabel, gFilterLabel)
# NOTE: we can't test visitInfo equality, because most fields are NaN.
self.assertIsNotNone(rtExposure.visitInfo)

def testSetMembers(self):
"""
Test that the MaskedImage and the WCS of an Exposure can be set.
Expand Down Expand Up @@ -495,6 +514,8 @@ def getExposure():
psf = readExposure.getPsf()
self.assertIsNotNone(psf)
self.assertEqual(psf, self.psf)
# check psf property getter
self.assertEqual(readExposure.psf, self.psf)

def checkWcs(self, parentExposure, subExposure):
"""Compare WCS at corner points of a sub-exposure and its parent exposure
Expand Down
4 changes: 2 additions & 2 deletions tests/test_imageHash.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ BOOST_AUTO_TEST_CASE(VisitInfoHash) {
45.1 * degrees, lsst::geom::SpherePoint(23.1 * degrees, 73.2 * degrees),
lsst::geom::SpherePoint(134.5 * degrees, 33.3 * degrees), 1.73, 73.2 * degrees,
RotType::SKY, coord::Observatory(11.1 * degrees, 22.2 * degrees, 0.333),
coord::Weather(1.1, 2.2, 34.5), "testCam1");
coord::Weather(1.1, 2.2, 34.5), "testCam1", 123456);
VisitInfo info2(10313423, 10.01, 11.02, DateTime(65321.1, DateTime::MJD, DateTime::TAI), 12345.1,
45.1 * degrees, lsst::geom::SpherePoint(23.1 * degrees, 73.2 * degrees),
lsst::geom::SpherePoint(134.5 * degrees, 33.3 * degrees), 1.73, 73.2 * degrees,
RotType::SKY, coord::Observatory(11.1 * degrees, 22.2 * degrees, 0.333),
coord::Weather(1.1, 2.2, 34.5), "testCam1");
coord::Weather(1.1, 2.2, 34.5), "testCam1", 123456);

utils::assertHashesEqual(info1, info2);
}
Expand Down