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

#2a - Creation of full_coupler_clocks object #106

Merged
merged 53 commits into from
May 30, 2024

Conversation

mlee03
Copy link
Contributor

@mlee03 mlee03 commented Mar 12, 2024

In this PR, a coupler_clocks object of type coupler_clock_type has been created and is used to replace the newClocks.

Because of slight changes in the order of clock id creations, the statistics of all clocks used in coupler will be outputted at the top of mpp_clock statistics after the outputs for

Total_runtime
Initialization
Main loop
Termination

AM4, AM5, SPEAR, and CM4 passes.

full/coupler_main.F90 Outdated Show resolved Hide resolved
full/coupler_main.F90 Outdated Show resolved Hide resolved
Comment on lines 407 to 408
character(len=128) :: version = '$Id$'
character(len=128) :: tag = '$Name$'
Copy link
Member

Choose a reason for hiding this comment

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

These should probably be added back in. There may be some models that use this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wondering if they wanted to use version and tag, they could use full_coupler_mod?

Copy link
Contributor

Choose a reason for hiding this comment

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

These are artifacts from our days of using CVS. We had a different method for GitHub, but trying to insert things was shell dependent and not reliable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like it is safe to remove then. I will remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spoke too soon, it is used in coupler_init.

Copy link
Contributor

Choose a reason for hiding this comment

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

Look at other files that use version and search back in their history to see how things have changed. For example here's mpp.F90 from 19 years ago and 9 years ago when version and tag where updated/removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do this in a separate PR

@mlee03 mlee03 marked this pull request as ready for review March 19, 2024 22:18
@mlee03 mlee03 changed the title Creation of full_coupler_clocks object #2 - Creation of full_coupler_clocks object Apr 17, 2024
@mlee03 mlee03 changed the title #2 - Creation of full_coupler_clocks object #2a - Creation of full_coupler_clocks object Apr 18, 2024
@mlee03
Copy link
Contributor Author

mlee03 commented Apr 18, 2024

@thomas-robinson @bensonr ready for review

@mlee03 mlee03 requested a review from uramirez8707 May 13, 2024 14:02
@@ -226,6 +228,47 @@ module full_coupler_mod
use_hyper_thread, concurrent_ice, slow_ice_with_ocean, &
do_endpoint_chksum, combined_ice_and_ocean

type coupler_clock_type
Copy link
Member

Choose a reason for hiding this comment

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

This needs documentation. The type and the variables.

@@ -226,6 +228,47 @@ module full_coupler_mod
use_hyper_thread, concurrent_ice, slow_ice_with_ocean, &
do_endpoint_chksum, combined_ice_and_ocean

type coupler_clock_type
integer :: initialization
Copy link
Member

Choose a reason for hiding this comment

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

These should all be private and we should use getters and setters to get and set the variables. Please open an issue to address this in the near future.

Comment on lines 1532 to 1539
type(coupler_clock_type), intent(inout) :: coupler_clocks
type(atmos_data_type), intent(in) :: Atm
type(land_data_type), intent(in) :: Land
type(ocean_public_type), intent(in) :: Ocean
type(ice_data_type), intent(in) :: Ice
integer, dimension(:), intent(in) :: slow_ice_ocean_pelist
integer, dimension(:,:), intent(in) :: ensemble_pelist
integer, intent(in) :: ensemble_id
Copy link
Member

Choose a reason for hiding this comment

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

These variables need documentation.

@@ -1503,5 +1523,107 @@ subroutine ocean_chksum(id, timestep, Ocean, Ice_ocean_boundary)

end subroutine ocean_chksum

!> \brief This subroutine sets the ID for clocks used in coupler_main
subroutine coupler_set_clock_ids(coupler_clocks, Atm, Land, Ice, Ocean, ensemble_pelist,&
Copy link
Member

Choose a reason for hiding this comment

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

This could be a type bound procedure. This is the setter. Please add it to the issue you open.

@rem1776 rem1776 merged commit 1fd282c into NOAA-GFDL:main May 30, 2024
2 checks passed
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.

5 participants