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

Update TOP solar radiation parameterization #6467

Merged

Conversation

daleihao
Copy link
Collaborator

@daleihao daleihao commented Jun 12, 2024

Minor modification for TOP solar radiation parameterization:
Use STD_ELEV std_elev when STDEV_ELEV is not available in fsurdata.

[BFB]

@rljacob
Copy link
Member

rljacob commented Jun 12, 2024

Does this change answers?

@rljacob rljacob added the Land label Jun 12, 2024
@daleihao
Copy link
Collaborator Author

Does this change answers?

No. It will affect the simulations only when TOP is turned on by use_top_solar_rad = .true..

@rljacob
Copy link
Member

rljacob commented Jun 12, 2024

But isn't TOP on by default? Its used in v3.

@daleihao
Copy link
Collaborator Author

daleihao commented Jun 12, 2024 via email

@rljacob
Copy link
Member

rljacob commented Jun 12, 2024

No it is on in the master branch. Our policy is that the v3.0 tag should make v3.0 answers without relying on a runscript. See #6108.

@@ -1811,7 +1811,7 @@ subroutine Albedo_TOP_Adjustment(bounds, num_pft, filter_pft, &
lon_180 = lon(g)
if (lon_180 > pi) lon_180 = lon_180-2._r8*pi

if (cosz > 0._r8 .and. abs(lat(g)) < 1.047_r8 .and. stdev_elev(g) > 0._r8) then
Copy link
Contributor

Choose a reason for hiding this comment

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

@daleihao, As we discussed offline, please undo this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Forgot to mention that once you revert this code change, please update the first comment msg. Thanks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@daleihao
Copy link
Collaborator Author

No it is on in the master branch. Our policy is that the v3.0 tag should make v3.0 answers without relying on a runscript. See #6108.

After undoing one change, the code will not change the answers.

@bishtgautam bishtgautam added the BFB PR leaves answers BFB label Jun 14, 2024
@rljacob
Copy link
Member

rljacob commented Jun 27, 2024

discussion: this makes sure TOP works for grids not used in WCYCLE v3.

@rljacob
Copy link
Member

rljacob commented Jul 25, 2024

@bishtgautam please re-review.

1 similar comment
@rljacob
Copy link
Member

rljacob commented Aug 20, 2024

@bishtgautam please re-review.

@bishtgautam bishtgautam self-requested a review August 20, 2024 21:41
@rljacob
Copy link
Member

rljacob commented Sep 4, 2024

@bishtgautam can this be merged?

@bishtgautam
Copy link
Contributor

I will merge this today.

bishtgautam added a commit that referenced this pull request Sep 4, 2024
Minor modification for TOP solar radiation parameterization:
Use `STD_ELEV` std_elev when `STDEV_ELEV` is not available in fsurdata.

[BFB]
@bishtgautam bishtgautam merged commit 003d81c into E3SM-Project:master Sep 5, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB Land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants