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

added gcc09 and vcg04 #13

Merged
merged 9 commits into from
Apr 23, 2020
Merged

added gcc09 and vcg04 #13

merged 9 commits into from
Apr 23, 2020

Conversation

siddharthlal25
Copy link
Member

@siddharthlal25 siddharthlal25 commented Apr 1, 2020

Added gcc09 and vcg04 as mentioned in #10.

@mileslucas
Copy link
Member

@giordano any clue why these travis builds are randomly failing?

@siddharthlal25 siddharthlal25 changed the title added gcc09 added gcc09 and vcg04 Apr 2, 2020
@codecov-io
Copy link

codecov-io commented Apr 2, 2020

Codecov Report

Merging #13 into master will increase coverage by 1.53%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
+ Coverage   96.15%   97.69%   +1.53%     
==========================================
  Files           3        3              
  Lines         104      130      +26     
==========================================
+ Hits          100      127      +27     
+ Misses          4        3       -1     
Impacted Files Coverage Δ
src/DustExtinction.jl 100.00% <ø> (+16.66%) ⬆️
src/color_laws.jl 95.65% <94.11%> (+2.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c05f1d...6aadd85. Read the comment docs.

@siddharthlal25 siddharthlal25 requested a review from giordano April 2, 2020 22:12
Copy link
Member

@mileslucas mileslucas left a comment

Choose a reason for hiding this comment

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

These are looking really good, and it's always great to add more models! Thanks. I think it's worth it to remove the polynomials within the invum functions, but keep the polynomial references in the ccm89. Basically, unless the polynomials are being passed to a funciton, we should use @evalpoly to avoid the allocation.

Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

Looks good to me now. @mileslucas?

@giordano
Copy link
Member

Cirrus is constantly failing across all projects I watch, I think we should just remove it...

@mileslucas
Copy link
Member

Looks good as far as I can tell too.

I guess let’s merge this and fix CI in another PR

@giordano
Copy link
Member

Shall we merge this?

@siddharthlal25
Copy link
Member Author

This PR is complete from my side. 😃

@giordano giordano merged commit 1766089 into JuliaAstro:master Apr 23, 2020
@mileslucas mileslucas mentioned this pull request Apr 24, 2020
10 tasks
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.

4 participants