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

compute apparent magnitudes from input catalog info #61

Merged
merged 2 commits into from
Aug 7, 2017

Conversation

jchiang87
Copy link
Collaborator

No description provided.

@cwwalter
Copy link
Member

cwwalter commented Aug 7, 2017

I was doing a check of old emails to make sure there was nothing I missed and came across this.

Somehow, I missed noticing it at the time. The code looks fine but I have a quick question:

I guess you used this in the pipeline to produce the true magnitudes in the star and galaxy catalog you made from the instance catalog. Is that right?

If we instead produce the catalogs directly from CatSim (like in LSSTDESC/SSim_DC1#41) would will still need this? And do you still want to add it any case?

@jchiang87
Copy link
Collaborator Author

I guess you used this in the pipeline to produce the true magnitudes in the star and galaxy catalog you made from the instance catalog. Is that right?

Yes, that's right.

If we instead produce the catalogs directly from CatSim (like in LSSTDESC/SSim_DC1#41) would will still need this? And do you still want to add it any case?

It doesn't hurt to merge anyways, even if it's functionality is superseded later.

@cwwalter
Copy link
Member

cwwalter commented Aug 7, 2017

OK, this looks good. I will merge.

@cwwalter cwwalter merged commit e280f2c into master Aug 7, 2017
@cwwalter cwwalter deleted the u/jchiang/compute_app_mags branch August 7, 2017 15:33
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.

2 participants