Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

Tickets/DM-13204: Create v14.0 versions of validation_data_decam #5

Merged
merged 6 commits into from Feb 6, 2018

Conversation

wmwv
Copy link
Contributor

@wmwv wmwv commented Jan 11, 2018

This is a reprocessed version of validation_data_decam with v14.0 of the DM stack. Note that this processing results in only 3 CCDs successfully processed for visit 0176837. Under the previous reference v12.0 processing all 9 CCDs considered were successfully processed. All CCDs are successfully processed for the second visit, 0176846.

@wmwv wmwv requested a review from r-owen January 11, 2018 13:51
Copy link

@r-owen r-owen left a comment

Choose a reason for hiding this comment

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

A few suggestions, the most important of which is to fix the instructions or the ups file so a user can set up the correct packages.

I am also uneasy about your use of setPhotocalConfigFromEups.

README.md Outdated
@@ -75,17 +75,18 @@ This repository was created using `examples/runDecamTest.sh` from the `validate_
To fully recreate this Butler `repo` from the `raw` data, set the `mapper` and add the `ingesetImages.py` step:

```
setup validation_data_decam
setup validation_data_decam -t v14_0
Copy link

Choose a reason for hiding this comment

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

This doesn't actually do anything because the ups table file is empty.

Please either fix the ups table file or these instructions so that the correct software is setup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Done.

config.calibrate.astromRefObjLoader.retarget(LoadAstrometryNetObjectsTask)
config.calibrate.photoRefObjLoader.retarget(LoadAstrometryNetObjectsTask)

setPhotocalConfigFromEups(config.calibrate.photoCal)
Copy link

Choose a reason for hiding this comment

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

LoadAstrometryNetObjectsTask is already the default loader, so I suggest removing the 3 relevant lines here (and perhaps in validate_drp as well).

Also I am puzzled by your use of setPhotocalConfigFromEups. Is that really wanted? I would expect the code to be far clearer if you just set the appropriate config options for the astrometry catalog you are using. That would also eliminate the need to set the environment variable ASTROMETRY_NET_DATA_DIR

Choose a reason for hiding this comment

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

setPhotocalConfigFromEups gets the astrometry reference catalog version and sets the photocal configuration appropriately. It is, unfortunately, currently standard; I believe @parejkoj may fix that though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the responses to RFC-433, I'm not expecting to make any changes to a.net refObjTask. Instead, I plan to replace all my existing a.net refcats with indexed PS1 or Gaia ones, and possibly file another RFC to remove the a.net ref loader.

There is no way that I'm aware of to setup LoadAstrometryNetObjectsTask other than the env variables: it doesn't even accept any config options!

@wmwv wmwv merged commit 5b4079e into master Feb 6, 2018
@wmwv
Copy link
Contributor Author

wmwv commented Feb 6, 2018

Thanks, @r-owen

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants