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

Read albedo from file over time (v2) #280

Merged
merged 1 commit into from
Aug 11, 2023
Merged

Read albedo from file over time (v2) #280

merged 1 commit into from
Aug 11, 2023

Conversation

juliasloan25
Copy link
Member

@juliasloan25 juliasloan25 commented Aug 1, 2023

Purpose

Using the recently-added FileReader module, this PR adds a new albedo model type that allows us to read in albedo from a NetCDF file that varies in time.

Closes #243

To-do

  • Copy infrastructure, tests from Read albedo over time (v1) #260
  • Update BulkAlbedoTemporal test to work
    • Add test for interpolate_data when current date = a date in the file
      • Maybe add a case to explicitly just return current data

Notes

  • For BulkAlbedoStatic, we use Regridder.MapInfo to store the albedo dataset file info. For BulkAlbedoTemporal, we use FileReader.PrescribedData. We can probably update BulkAlbedoStatic to use PrescribedData instead, or one of the component structs of PrescribedData (FileInfo?)
  • This PR introduces a test for BulkAlbedoTemporal in test/standalone/Bucket/albedo_types.jl, which also tests some functionality of FileReader.interpolate_data that isn't tested elsewhere. This test involves a lot of setup (making a PrescribedData object and everything inside of it, reading in a data file for many points in time, creating a space) which is required for the BulkAlbedoTemporal test anyway, so it was easier to just write it there. However, we should really add a more thorough test for interpolate_data in test/shared_utilities/file_reader.jl so the module's functions are thoroughly tested there.

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

@juliasloan25 juliasloan25 changed the title Read albedo from file over time Read albedo from file over time (v2) Aug 1, 2023
@juliasloan25 juliasloan25 requested a review from kmdeck August 3, 2023 21:21
@juliasloan25 juliasloan25 self-assigned this Aug 3, 2023
@juliasloan25 juliasloan25 mentioned this pull request Aug 4, 2023
1 task
@kmdeck
Copy link
Member

kmdeck commented Aug 4, 2023

you wrote:

For BulkAlbedoStatic, we use [Regridder.MapInfo](https://github.com/CliMA/ClimaLSM.jl/blob/main/src/shared_utilities/Regridder.jl#L357) to store the albedo dataset file info. For BulkAlbedoTemporal, we use [FileReader.PrescribedData](https://github.com/CliMA/ClimaLSM.jl/blob/main/src/shared_utilities/FileReader.jl#L96). We can probably update BulkAlbedoStatic to use PrescribedData instead, or one of the component structs of PrescribedData (FileInfo?)

    Maybe in a separate PR we can work on simplify these to just use one struct

this is a great idea.

src/standalone/Bucket/Bucket.jl Outdated Show resolved Hide resolved
src/standalone/Bucket/Bucket.jl Outdated Show resolved Hide resolved
src/standalone/Bucket/Bucket.jl Show resolved Hide resolved
src/standalone/Bucket/Bucket.jl Show resolved Hide resolved
src/standalone/Bucket/Bucket.jl Outdated Show resolved Hide resolved
src/standalone/Bucket/Bucket.jl Outdated Show resolved Hide resolved
src/shared_utilities/FileReader.jl Outdated Show resolved Hide resolved
Y.bucket.W .= 0.0
Y.bucket.Ws .= 0.0
Y.bucket.σS .= 0.0
set_initial_aux_state! = make_set_initial_aux_state(model)
Copy link
Member

Choose a reason for hiding this comment

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

perhaps we can also test set_initial_parameter_field! and next_albedo directly (aside from via update_aux and set_initial_aux_state)

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, good idea

@juliasloan25
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 11, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 81af155 into main Aug 11, 2023
@bors bors bot deleted the js/albedo-time2 branch August 11, 2023 20:49
mitraA90 pushed a commit that referenced this pull request Dec 22, 2023
280: Read albedo from file over time (v2) r=juliasloan25 a=juliasloan25



Co-authored-by: Julia Sloan <jsloan@caltech.edu>
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.

Time-varying albedo
2 participants