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

Add constant snow albedo as an option #235

Merged
merged 3 commits into from
Feb 14, 2024
Merged

Add constant snow albedo as an option #235

merged 3 commits into from
Feb 14, 2024

Conversation

gaobhub
Copy link
Contributor

@gaobhub gaobhub commented Feb 7, 2024

No description provided.

Copy link
Collaborator

@ecoon ecoon left a comment

Choose a reason for hiding this comment

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

Please parse the list beforehand -- calling isParameter() and get() in the innermost loop is a bad idea for performance reasons.

If you parse the list in the constructor, then have both a boolean, e.g.

bool is_constant_snow_albedo_ = plist_.isParameter(...);
double snow_albedo_ = plist_.get<double>(...);

Also, only add snow_dens to the list of dependencies if !is_constant_snow_albedo_ which will require some changes in the Evaluate method to only get and use the vector when you actually have it as a dependency, something like

Epetra_MultiVector* dens_snow = nullptr;
if (!is_constant_albedo_snow_) {
    dens_snow = S_->Get(dens_snow_key_,...)->ViewComponent("cell").get();
}

Then, here you'll have something like:

albedo[2][c] = is_constant_snow_albedo_ ? snow_albedo_ : Relations::CalcSnowAlbedo((*snow_dens)[0][c]);

@@ -81,7 +85,7 @@ AlbedoThreeComponentEvaluator::Evaluate_(const State& S,

for (auto c : lc_ids) {
// albedo of the snow
albedo[2][c] = Relations::CalcAlbedoSnow(snow_dens[0][c]);
albedo[2][c] = is_constant_snow_albedo_ ? plist_.get<double>("albedo snow [-]") : Relations::CalcAlbedoSnow((*snow_dens)[0][c]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

No, also need to take this out. All parsing of the ParameterList must be done in the constructor. Also add

albedo_snow_ = plist_.get<double>("...");

in the constructor, then just use it here.

@ecoon ecoon merged commit 26799fb into master Feb 14, 2024
1 check passed
@ecoon ecoon deleted the gaob/albedo branch February 14, 2024 22:06
levuvietphong pushed a commit that referenced this pull request Feb 17, 2024
* Add constant snow albedo as an option

* Add constant snow albedo as an option; use snow density as a conditional dependency; rename snow emissivity

* Add the option of asking for constant snow albedo to constructor
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