Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw
-
Labels:
-
Story Points:5
-
Epic Link:
-
Sprint:AP F18-2
-
Team:Alert Production
Description
Change fluxSigma to fluxErr, apCorrSigma to apCorrErr and similarly for the diagonal elements of covariance matrices. This includes variable names, argument names and afw table field names. This is necessary in order to implement RFC-333.
Change afw::table::CovarianceMatrixKey to use "Err" as a suffix rather than "Sigma".
Update the FITS catalog persistence as follows:
- Bump the schema VERSION constant from 1 to 2 in SchemaImpl.h
- When reading version 1 catalogs make a fooErr as an alias to every fooSigma field
- When reading version 0 catalogs make Err aliases instead of Sigma aliases.
Attachments
Issue Links
- blocks
-
DM-8828 Support proper motions in reference catalogs
- Done
- duplicates
-
- Won't Fix
- is triggered by
-
- Implemented
- is triggering
-
DM-15355 ci_hsc failure: aperture correction fields for PsfFlux not present
- Done
Activity
| Field | Original Value | New Value |
|---|---|---|
| Epic Link |
|
| Risk Score | 0 |
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Summary | Add support for "Err" suffix to CovarianceMatrixKey | Change CovarianceMatrixKey field name suffix from Sigma to Err |
| Description |
{{afw::table::CovarianceMatrixKey}} should support "Err" as a suffix, as well as "Sigma". This is necessary in order to implement Simply allowing this suffix to be specified is probably trivial. However, for backwards compatibility it would be nice if we could read existing persisted tables with "Sigma" suffix as "Err", but I have no idea how practical that is. [~jbosch] do you have any ideas about this? My initial estimate of story points is based on either not implementing backwards compatibility or having it be fairly easy. |
{{afw::table::CovarianceMatrixKey}} should use "Err" instead of "Sigma" as its field name suffix for diagonal elements. This is necessary in order to implement Update the VERSION in SchemaImpl.h and add code that handles older catalogs. |
| Description |
{{afw::table::CovarianceMatrixKey}} should use "Err" instead of "Sigma" as its field name suffix for diagonal elements. This is necessary in order to implement Update the VERSION in SchemaImpl.h and add code that handles older catalogs. |
{{afw::table::CovarianceMatrixKey}} should support "Err" as a suffix, as well as "Sigma". This is necessary in order to implement Simply allowing this suffix to be specified is probably trivial. However, for backwards compatibility it would be nice if we could read existing persisted tables with "Sigma" suffix as "Err", but I have no idea how practical that is. [~jbosch] do you have any ideas about this? My initial estimate of story points is based on either not implementing backwards compatibility or having it be fairly easy. I also see that DM- |
| Story Points | 2 | 5 |
| Summary | Change CovarianceMatrixKey field name suffix from Sigma to Err | Change fluxSigma to fluxErr and similarly for covariances |
| Description |
{{afw::table::CovarianceMatrixKey}} should support "Err" as a suffix, as well as "Sigma". This is necessary in order to implement Simply allowing this suffix to be specified is probably trivial. However, for backwards compatibility it would be nice if we could read existing persisted tables with "Sigma" suffix as "Err", but I have no idea how practical that is. [~jbosch] do you have any ideas about this? My initial estimate of story points is based on either not implementing backwards compatibility or having it be fairly easy. I also see that DM- |
Change fluxSigma to fluxErr, apCorrSigma to apCorrErr and similarly for the diagonal elements of covariance matrices. This includes variable names, argument names and afw table field names. This is necessary in order to implement Change {{afw::table::CovarianceMatrixKey}} to use "Err" as a suffix rather than "Sigma". Update the FITS catalog persistence as follows: - Bump the schema VERSION constant from 1 to 2 in SchemaImpl.h - When reading version 1 catalogs make a {{fooErr}} as an alias to every {{fooSigma}} field - When reading version 0 catalogs make {{Err}} aliases instead of {{Sigma}} aliases. |
| Summary | Change fluxSigma to fluxErr and similarly for covariances | Change fluxSigma to fluxErr and similarly for apCorr and covariances |
Jim Bosch please review the changes in afw. Sophie Reed please look at the others. If you need help let me know – I am happy to answer questions and if it's too much work I can ask somebody else to review some of the packages.
| Reviewers | Jim Bosch [ jbosch ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
| Reviewers | Jim Bosch [ jbosch ] | Jim Bosch, Sophie Reed [ jbosch, sophiereed ] |
My review is complete.
Notes:
- jointcal has some jupyter notebooks with code that uses the old field names. I looked into updating those notebooks, but they are not python 3 compatible (due to old style print statements) and the ones that need updating have hard-coded paths that are relevant to a particular user, so I gave up and left them alone. John Parejko may wish to weigh in on this.
- cat has a lot of sql with the old names. I'm not sure what to do with it.
- qserv has a lot of code with the old names. I've warned Fritz Mueller. He said old tables can be left alone.
- qa_explorer has a jupyter notebook with the old field names as output of one cell. This should not stop anything in the notebook from running and I could not update the notebook because I could not get display_ginga installed.
The old jointcal notebooks I'm keeping around for reference purposes (in particular, Astrometry.ipynb, CheckSimAstrom.ipynb, and checkCoadd.ipynb, all of which came with the original meas_simastrom). They don't work for other reasons too (e.g., jointcal's debug output having changed), but they have been useful for coming up with ideas for debugging and plots.
I have looked at all the other pull requests, apart from the one I commented on they all look good.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
I've checked the new obs_subaru one and it looks fine.
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
Thank you very much for the reviews.
I wrote a short community post describing the changes.
JIRA is not showing branches or pull requests for afw and pipe_tasks, as usual, so here they are:
The reason there is one more branch than pull request is that JIRA is showing an extra, unrelated branch for obs_subaru (in addition to the correct branch). A bug that's new to me:
DM-15273