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

integrate neural network into Snow.jl architecture #905

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

a-charbon
Copy link
Contributor

Purpose

Integrate the neural snow depth model into the Snow model architecture. This involves small changes to Snow.jl and SnowParameterizations.jl as well as expanding the NeuralSnow extension to allow the neural model as a type of AbstractDensityModel. "CSV" was also added to the buildkite dependencies to permit testing of the NeuralSnow extension, and BSON has been appropriately added to the ClimaLand weak dependencies to enable the expanded functionality of the extension.

To-do

Questions are commented in the changed files about different possible approaches. With this PR merged, I can also add a slightly faster and more abstract/streamlined form of the neural model itself (changes to ModelTools, etc).

Content

Expanded NeuralSnow extension and changed some of the syntax of the Snow model.


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

@a-charbon a-charbon requested a review from kmdeck November 1, 2024 00:30
@@ -57,6 +69,8 @@ Y, p, coords = ClimaLand.initialize(model)

# Set initial conditions
Y.snow.S .= FT(SWE[1]) # first data point
#Y.snow.Z .= FT(depths[1]) #uncomment if using dens_neural, not dens_constant
p.snow.ρ_snow .= FT(SWE[1] / depths[1] * 1000.0)
Copy link
Member

Choose a reason for hiding this comment

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

you dont need to set this, it is set in set_initial_cache!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to myself to check a model simulation without this line, I think I ran into problems without it

ModelTools.settimescale!(zmodel, 86400.0)
return zmodel

#should I write this function to give the ability to also load from local file too?
Copy link
Member

Choose a reason for hiding this comment

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

our style is more to have the download link download the file, managed by ClimaArtifacts, and then to load from file.

Perhaps you could switch to that style (read from local file), and open an issue in ClimaArtifacts to add this? we can resolve that later, though


end

#do tests need to be made for these functions? since it is an extension?
Copy link
Member

Choose a reason for hiding this comment

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

we have neural snow model extension tests currently:
https://github.com/CliMA/ClimaLand.jl/blob/main/test/standalone/Snow/tool_tests.jl

it would be good to add some tests for these functions

p,
)
#Get Inputs:
#is this too many allocations?
Copy link
Member

Choose a reason for hiding this comment

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

yes, anything with a dot (broadcast) over a field needs to be in an expression that involves x .= ...

so that there are no allocations.

You can do this like:

    @. dY.snow.P_avg = β * (abs(p.drivers.P_snow) .- Y.snow.P_avg)
    @. dY.snow.T_avg = β * (p.drivers.T - model.parameters.earth_param_set.T_freeze - Y.snow.T_avg)
    @. dY.snow.R_avg = β * (p.drivers.SW_d - Y.snow.R_avg) # SW_d is positive by definition
    @. dY.snow.Qrel_avg = β * (Thermodynamics.relative_humidity(
            Ref(model.boundary_conditions.atmos.thermo_params),
            p.drivers.thermal_state,
        ) - Y.snow.Qrel_avg)
    @. dY.snow.u_avg = β * (wind_speed - Y.snow.u_avg)

the way it is written, it is not possible to do this for dY.snow.Z. But you could add in an aux var

 p.snow.predicted_dzdt .=  dzdt(density, Y)

which is updated here (if you like) and then compute

@. dY.snow.Z = clip_dZdt(swe, z, dswedt, predicted_dzdt, dt)

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