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.

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-24319: Add support for new parquet source tables in fgcmcal #296

Merged
merged 1 commit into from
Jul 3, 2020

Conversation

erykoff
Copy link
Contributor

@erykoff erykoff commented Jun 15, 2020

No description provided.

Copy link
Contributor

@parejkoj parejkoj left a comment

Choose a reason for hiding this comment

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

Please add comments describing configuration options. Why minPerBand=2? Why that fluxField and SN cut?

Otherwise, it becomes much harder for future people to figure out why a given instrument-specific configuration exists.

config.doSubtractLocalBackground = True

config.fgcmLoadReferenceCatalog.refObjLoader.ref_dataset_name = 'ps1_pv3_3pi_20170110'
config.fgcmLoadReferenceCatalog.refFilterMap = {'g': 'g', 'r': 'r', 'r2': 'r',
Copy link
Contributor

Choose a reason for hiding this comment

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

I was confused by this line and the above filterMap because I believe our LoadReferenceObjectsTask does not need matching filter names to be defined in the filterMap (e.g. how our PS1 refcats work, because most filter names exist in both). Maybe add a comment to that effect, because this is not the same thing?

config.fgcmOutputProducts.load(os.path.join(os.path.dirname(__file__),
'fgcmOutputProducts.py'))
config.fgcmFitCycle.aperCorrFitNBins = 0
config.fgcmFitCycle.useRepeatabilityForExpGrayCutsDict = {'N387': True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this "all filters True" or "just these specific filters True"? It's not clear from how the list is made: maybe there's a better way to make that clear? I don't know if using the FilterDefinitions in hscFilters.py might help?

Copy link
Contributor

@parejkoj parejkoj left a comment

Choose a reason for hiding this comment

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

Thank you for the added comments: this will help future users immensly.

@erykoff erykoff merged commit d8347dd into master Jul 3, 2020
@erykoff erykoff deleted the tickets/DM-24319 branch July 3, 2020 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants