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

In UrbanAlbedo: Create a filter for coszen>0 and operate over that #747

Open
billsacks opened this issue Jun 13, 2019 · 2 comments · May be fixed by #2893
Open

In UrbanAlbedo: Create a filter for coszen>0 and operate over that #747

billsacks opened this issue Jun 13, 2019 · 2 comments · May be fixed by #2893
Assignees
Labels
blocker another issue/PR depends on this one enhancement new capability or improved behavior of existing capability performance idea or PR to improve performance (e.g. throughput, memory)

Comments

@billsacks
Copy link
Member

UrbanAlbedo currently calculates a variable num_solar, and then there is a big block of code that is only done if num_solar > 0:
https://github.com/ESCOMP/ctsm/blob/2632be6af65ad7d1c0d23ae501c76b3cf68d0d6c/src/biogeophys/UrbanAlbedoMod.F90#L264-L429

This logic causes a dependence of some diagnostic fields on decomposition (#17). Moreover, while the current logic avoids doing any work if num_solar == 0, it is less efficient than it could be in the (probably common) case where num_solar > 0 but < num_urban.

So for the sake of both fixing this dependence on decomposition and performance, we should introduce a filter in this routine (which is also passed down to the routines it calls) that holds just the points with coszen>0. Then we should operate over that filter, and remove the num_solar > 0 conditional.

Blocks #17 .

@billsacks billsacks added enhancement new capability or improved behavior of existing capability blocker another issue/PR depends on this one labels Jun 13, 2019
@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Jan 9, 2023
@billsacks billsacks removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Mar 2, 2023
@samsrabin samsrabin added the performance idea or PR to improve performance (e.g. throughput, memory) label Feb 8, 2024
@olyson olyson self-assigned this Oct 16, 2024
@olyson
Copy link
Contributor

olyson commented Oct 16, 2024

@keerzhang1 was looking at this code recently and brought up this same issue, so I'll take a look at this soon.

@olyson
Copy link
Contributor

olyson commented Nov 5, 2024

I've implemented changes to address these two issues (this one here and #17 ). aux_clm testing shows that these are bfb with the two problematic history fields active by default (ALBGRD and ALBGRI). I will issue a PR once HPC systems come back online. There may be further improvements that can be made regarding the performance aspect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker another issue/PR depends on this one enhancement new capability or improved behavior of existing capability performance idea or PR to improve performance (e.g. throughput, memory)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants