Skip to content

Add default handling for fates firemodes in lightning resolution #3606

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: b4b-dev
Choose a base branch
from
Open

Add default handling for fates firemodes in lightning resolution #3606

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04d6941
add default handling for fates firemodes in lightning resolution
mvdebolskiy Nov 12, 2025
c3cb928
add checks for the vars not being already defined.
mvdebolskiy Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 24 additions & 16 deletions bld/CLMBuildNamelist.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ sub setup_cmdl_fates_mode {
"use_fates_daylength_factor", "fates_photosynth_acclimation", "fates_stomatal_model",
"fates_stomatal_assimilation", "fates_leafresp_model", "fates_cstarvation_model",
"fates_regeneration_model", "fates_hydro_solver", "fates_radiation_model", "fates_electron_transport_model",
"use_fates_managed_fire"
"use_fates_managed_fire"
);

# dis-allow fates specific namelist items with non-fates runs
Expand Down Expand Up @@ -1027,6 +1027,27 @@ sub setup_cmdl_fire_light_res {
if ( &value_is_true($nl->get_value('use_cn')) ) {
add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'fire_method');
}

Copy link
Collaborator

@ekluzek ekluzek Nov 13, 2025

Choose a reason for hiding this comment

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

I think it makes sense for this to go here. But, I also think that the call to setup_logic_fates could be moved in front of this as an alternative way to do it. Since, the lightning resolution for fire is now used for both BGC and FATES I think we should add more comments to the header about this being the case. This was originally designed as only being for BGC and not FATES. So it would be good to clarify that in the header and some of the comments in the code.

Copy link
Collaborator

@ekluzek ekluzek Nov 13, 2025

Choose a reason for hiding this comment

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

I was thinking the order could be changed, but fates_spitfire_mode isn't set until much later. So I don't see a way to merely change the order of the calls to make this work. You might have already tested that as the easiest solution.

In any case since both FATES and BGC need to determine the lightning resolution, they both need to be taken into account here so this change makes sense.

Copy link
Collaborator

@ekluzek ekluzek Nov 14, 2025

Choose a reason for hiding this comment

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

The other thing I realized is that to protect against reordering going wrong -- we can add some assert statements to the subroutines to ensure the needed things are appropriately set. It would be simple to add a few statements to start doing that.

Copy link
Collaborator

@ekluzek ekluzek Nov 14, 2025

Choose a reason for hiding this comment

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

So @mvdebolskiy there's a couple small things that I can see doing that would be good to bring in with this. I suggest the way forward, would be for me to push to your branch -- but have you look over my changes at the end and make sure you approve. I could also give you some instructions and have you implement it -- and then go back and forth to get it right. But, I think it would be easier to have me do those couple things here. So how does that sound for process?

# we have to process defaults for fates fire modes before and not in setup_logc_fates, however, they should not be defined for use_cn or other bgc
if ( &value_is_true($nl->get_value('use_fates')) ) {
if ( ! defined($nl->get_value('use_fates_managed_fire')) ) {
Copy link
Collaborator

@ekluzek ekluzek Nov 17, 2025

Choose a reason for hiding this comment

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

This line here and in 1037 can be removed.

add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'use_fates_managed_fire',
'use_fates'=>$nl_flags->{'use_fates'}, 'use_fates_sp'=>$nl_flags->{'use_fates_sp'} );
}
if ( ! defined($nl->get_value('fates_spitfire_mode')) ) {
add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'fates_spitfire_mode', 'use_fates'=>$nl_flags->{'use_fates'},
'use_fates_managed_fire'=>$nl->get_value('use_fates_managed_fire'), 'use_fates_sp'=>$nl_flags->{'use_fates_sp'} );
}
# Check use_fates_managed_fire mode is running with spitfire on
if ( defined($nl->get_value('use_fates_managed_fire')) ) {
if ( &value_is_true($nl->get_value('use_fates_managed_fire')) ) {
if ( $nl->get_value('fates_spitfire_mode') == 0 ) {
$log->fatal_error("fates_spitfire_mode must be non-zero when use_fates_managed_fire is true");
}
}
}
}

my $fire_method = remove_leading_and_trailing_quotes( $nl->get_value('fire_method') );
if ( $val eq "default" ) {
add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, $var,
Expand Down Expand Up @@ -4781,8 +4802,7 @@ sub setup_logic_fates {
"fates_harvest_mode","fates_parteh_mode", "use_fates_cohort_age_tracking","use_fates_tree_damage",
"use_fates_daylength_factor", "fates_photosynth_acclimation", "fates_stomatal_model",
"fates_stomatal_assimilation", "fates_leafresp_model", "fates_cstarvation_model",
"fates_regeneration_model", "fates_hydro_solver", "fates_radiation_model", "fates_electron_transport_model",
"use_fates_managed_fire"
"fates_regeneration_model", "fates_hydro_solver", "fates_radiation_model", "fates_electron_transport_model"
);

foreach my $var ( @list ) {
Expand All @@ -4802,9 +4822,7 @@ sub setup_logic_fates {
add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'use_fates_fixed_biogeog', 'use_fates'=>$nl_flags->{'use_fates'},
'use_fates_lupft'=>$nl->get_value('use_fates_lupft'),
'use_fates_sp'=>$nl_flags->{'use_fates_sp'} );
add_default($opts, $nl_flags->{'inputdata_rootdir'}, $definition, $defaults, $nl, 'fates_spitfire_mode', 'use_fates'=>$nl_flags->{'use_fates'},
'use_fates_managed_fire'=>$nl->get_value('use_fates_managed_fire'),
'use_fates_sp'=>$nl_flags->{'use_fates_sp'} );


my $suplnitro = $nl->get_value('suplnitro');
my $parteh_mode = $nl->get_value('fates_parteh_mode');
Expand Down Expand Up @@ -4939,16 +4957,6 @@ sub setup_logic_fates {
}
}
}

# Check use_fates_managed_fire mode is running with spitfire on
my $var = "use_fates_managed_fire";
if ( defined($nl->get_value($var)) ) {
if ( &value_is_true($nl->get_value($var)) ) {
if ( $nl->get_value('fates_spitfire_mode') == 0 ) {
$log->fatal_error("fates_spitfire_mode must be non-zero when $var is true");
}
}
}
}
}

Expand Down