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.

Sign up for GitHub

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

Jump to bottom

Refactor interface for instantaneous_zenith_angle function #60

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Refactor interface for instantaneous_zenith_angle function #60

merged 1 commit into from
Nov 9, 2023

Conversation

sriharshakandala
Copy link
Member

@sriharshakandala sriharshakandala commented Nov 6, 2023 •
edited
Loading

Purpose

Refactor interface for instantaneous_zenith_angle function
Tag v0.8.0

  • I have read and checked the items on the review checklist.

@sriharshakandala sriharshakandala mentioned this pull request Nov 6, 2023
Closed
@codecov Codecov
Copy link

codecov bot commented Nov 6, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3130c2c) 99.00% compared to head (7e936a4) 98.93%.
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
- Coverage   99.00%   98.93%   -0.07%     
==========================================
  Files           4        4              
  Lines         100       94       -6     
==========================================
- Hits           99       93       -6     
  Misses          1        1
Files Coverage Δ
src/InsolationCalc.jl 100.00% <100.00%> (ø)
src/ZenithAngleCalc.jl 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sriharshakandala sriharshakandala marked this pull request as ready for review November 6, 2023 23:51
szy21
szy21 approved these changes Nov 7, 2023
View reviewed changes
src/ZenithAngleCalc.jl Outdated Show resolved Hide resolved
@sriharshakandala
Copy link
Member Author

bors r+

1 similar comment
@sriharshakandala
Copy link
Member Author

bors r+

simonbyrne
simonbyrne reviewed Nov 7, 2023
View reviewed changes
src/ZenithAngleCalc.jl Outdated Show resolved Hide resolved
simonbyrne
simonbyrne reviewed Nov 7, 2023
View reviewed changes
src/ZenithAngleCalc.jl Outdated Show resolved Hide resolved
@sriharshakandala
Copy link
Member Author

sriharshakandala commented Nov 7, 2023 •
edited
Loading

@simonbyrne : Is this interface reasonable?
d can be omitted here. I currently left this in to stay consistent with the current return type! I can remove this if it is not needed! cc: @szy21

szy21
szy21 reviewed Nov 7, 2023
View reviewed changes
src/ZenithAngleCalc.jl Outdated Show resolved Hide resolved
@simonbyrne
Copy link
Member

from what i can tell, it looks fine, but i would suggest making the corresponding changes to ClimaAtmos, and see if it runs

@sriharshakandala
Copy link
Member Author

from what i can tell, it looks fine, but i would suggest making the corresponding changes to ClimaAtmos, and see if it runs

Yes. I am already working on that!

@sriharshakandala
Copy link
Member Author

I tried this locally on the cluster and it works!
I made a draft PR at CliMA/ClimaAtmos.jl#2341
I started the GPU pipeline here: https://buildkite.com/clima/climaatmos-target-gpu-simulations/builds/134

@sriharshakandala
Copy link
Member Author

It runs to completion on the GPU pipeline here: https://buildkite.com/clima/climaatmos-target-gpu-simulations/builds/138#018bad4c-6dfa-42fa-89fc-48292ca85d8b

@sriharshakandala
Copy link
Member Author

I am planning to merge unless there are any additional comments!

@simonbyrne
Copy link
Member

Bump the version?

@sriharshakandala
Copy link
Member Author

Bump the version?

Done.

simonbyrne
simonbyrne reviewed Nov 8, 2023
View reviewed changes
test/runtests.jl Outdated Show resolved Hide resolved
simonbyrne
simonbyrne reviewed Nov 8, 2023
View reviewed changes
src/ZenithAngleCalc.jl Outdated Show resolved Hide resolved
simonbyrne
simonbyrne reviewed Nov 8, 2023
View reviewed changes
src/ZenithAngleCalc.jl Outdated Show resolved Hide resolved
simonbyrne
simonbyrne reviewed Nov 8, 2023
View reviewed changes
src/ZenithAngleCalc.jl Outdated Show resolved Hide resolved
@sriharshakandala sriharshakandala force-pushed the sk/pass_date0 branch 2 times, most recently from 08363be to f4b9798 Compare November 8, 2023 23:10
@sriharshakandala sriharshakandala changed the title Pass in date0 as an argument. Refactor interface for instantaneous_zenith_angle function Nov 8, 2023
@sriharshakandala sriharshakandala merged commit f7be333 into main Nov 9, 2023
8 checks passed
@sriharshakandala sriharshakandala linked an issue Nov 9, 2023 that may be closed by this pull request
Update instantaneous zenith angle computation #57
Closed
@sriharshakandala sriharshakandala mentioned this pull request Nov 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szy21 szy21 szy21 approved these changes

@simonbyrne simonbyrne simonbyrne left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update instantaneous zenith angle computation
3 participants
@sriharshakandala @simonbyrne @szy21