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

Copy derivative of Planck formula from python pixell #53

Merged
merged 5 commits into from
Aug 20, 2022

Conversation

louisbranch
Copy link
Contributor

I am working on a project with ACTpol data in Julia, supervised by @guanyilun, that requires the derivative of Planck spectrum. I copied the function and the relevant constants from the https://github.com/simonsobs/pixell

Please let me know if there are contribution guidelines I should be following.

src/utils.jl Outdated
const h = 6.62606957e-34
const k = 1.3806488e-23
const T_cmb = 2.72548 # +/- 0.00057
const c = 299792458.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm leary of global constants which are single-letter lowercase. It's really easy to write a for loop with k as the iteration variable, for example. Can you make these local to your function for now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it might be nice to use a dependency to the PhysicalConstants package but that doesn't need to happen in this PR.

Copy link
Contributor Author

@louisbranch louisbranch Aug 19, 2022

Choose a reason for hiding this comment

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

@xzackli Thank you for the review and sorry for the delay. I have included the PhysicalConstants dependency.

@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2022

Codecov Report

Merging #53 (230b885) into main (c6d591b) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #53      +/-   ##
==========================================
+ Coverage   89.23%   89.32%   +0.09%     
==========================================
  Files           7        8       +1     
  Lines         808      815       +7     
==========================================
+ Hits          721      728       +7     
  Misses         87       87              
Impacted Files Coverage Δ
src/Pixell.jl 100.00% <ø> (ø)
src/utils.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@xzackli
Copy link
Collaborator

xzackli commented Aug 20, 2022

Looks good to me, I'm happy to merge this. Thanks for this nice addition!

@xzackli xzackli merged commit 4e6fcc9 into simonsobs:main Aug 20, 2022
@louisbranch louisbranch deleted the dplanck branch September 1, 2022 12:23
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.

3 participants