initialized_set_block bug fix

[grantfirl]

When trying to update the latest UFS to use the latest CCPP framework hash, one of the regression tests using CMEPS uses a suite that doesn't have any schemes with a non-empty init phase. Due to this, the subroutine generated for the init phase in the cap doesn't have access to a ccpp_t instance. Yet, in mkstatic.py a line like initialized({ccpp_var_name}%ccpp_instance) = .true. is put in the subroutine regardless, resulting in a compilation error.

A fix is to pass the ccpp_t_instance into the group cap for the init phase when it is otherwise empty so that initialized({ccpp_var_name}%ccpp_instance) = .true. has access to the ccpp_t_instance.

User interface changes?: No

Fixes: No issue started

Testing:
test removed:
unit tests:
system tests: UFS RTs (re-running as of 2023/10/12), SCM RTs (completed successfully)
manual testing:

[grantfirl]

@mkavulich @climbfuji @gold2718 This is a bugfix for #463 and this must be merged before the UFS can use the latest ccpp-framework commit.

[climbfuji]

I think that's ok, but I am not entirely familiar with how the NRL are using the member functionality (for which initialized was turned into an array). Is it potentially dangerous to set all members to initialize because we don't have enough information on the member for which this is called?

It would have been more along the lines of ccpp_prebuild if the initialized_set_block was a formatted string like the others (Group.initialized_set_blocks['fallback'] maybe, same for test_blocks)?

[climbfuji]

I think that's ok, but I am not entirely familiar with how the NRL are using the member functionality (for which initialized was turned into an array). Is it potentially dangerous to set all members to initialize because we don't have enough information on the member for which this is called?

It would have been more along the lines of ccpp_prebuild if the initialized_set_block was a formatted string like the others (Group.initialized_set_blocks['fallback'] maybe, same for test_blocks)?

@michalakes can you comment please?

[gold2718]

This is one of those weird times when I think I understand the fix but no do not understand how it ever worked before.

[michalakes]

I think that's ok, but I am not entirely familiar with how the NRL are using the member functionality (for which initialized was turned into an array). Is it potentially dangerous to set all members to initialize because we don't have enough information on the member for which this is called?
It would have been more along the lines of ccpp_prebuild if the initialized_set_block was a formatted string like the others (Group.initialized_set_blocks['fallback'] maybe, same for test_blocks)?

@michalakes can you comment please?

I'm out of the office until next week but I'll test to make sure this proposed fix does no harm for the NRL codes and let you know.

[grantfirl]

I think that's ok, but I am not entirely familiar with how the NRL are using the member functionality (for which initialized was turned into an array). Is it potentially dangerous to set all members to initialize because we don't have enough information on the member for which this is called?

It would have been more along the lines of ccpp_prebuild if the initialized_set_block was a formatted string like the others (Group.initialized_set_blocks['fallback'] maybe, same for test_blocks)?

You make a really good point about the danger of setting the entire initialized array. While it fixes the UFS problem, it may introduce another for NRL. I've updated the fix to just pass in the ccpp_t instance (cdata) to the init subroutine in the group cap (when it is otherwise empty) so that it can properly be referenced by the initialized set statement as originally intended. I think that this should be a better solution.

I'm re-testing in UFS to make sure, but @climbfuji @michalakes @gold2718 please take a look at the updated fix when you get a chance.

[climbfuji]

Wow, that's a lot cleaner and safer in my opinion. Thanks for considering my comments on the original version.

[grantfirl]

Passed UFS regression test that was failing initially. I'm rerunning the rest now...

[grantfirl]

Passed all UFS RTs on Hera except control_p8_gnu which is likely a different, unrelated issue: ufs-community/ufs-weather-model#1939

[mkavulich]

@michalakes Would you be able to test the latest fixes here? Just ensuring this won't cause any problems on the NRL side.

[michalakes]

@michalakes Would you be able to test the latest fixes here? Just ensuring this won't cause any problems on the NRL side.

Yes, sorry. I got sidetracked this week. Please stand by...

[michalakes]

I tested fix with NEPTUNE and "does no harm". I wasn't able to update to the latest CCPP because of some other issues in progress, but I spliced in the set of changes to external/ccpp/ccpp-framework/scripts/mkstatic.py and tested with one of the cases we have that exercise two domains. That worked as before. So... thumbs up from our end. Thanks very much for checking with us on this! And sorry for the delay. -John

[mkavulich]

Thanks for confirming @michalakes! Merging now.