Skip to content

Cleans up LIGHT data structures for greater efficiency #32

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

Cleans up LIGHT data structures for greater efficiency #32

wants to merge 0 commits into from

Conversation

pwolfram
Copy link
Contributor

@pwolfram pwolfram commented May 1, 2018 •
edited
Loading

Ocean/light restart cleanup

Removes code related to online autocorrelation computation, which is not used in practice.
Code cleanup removes communication needed for each particle for enhanced efficiency.
Results should be BFB under the current test cases.

@pwolfram pwolfram changed the base branch from master to develop May 1, 2018 19:36
@pwolfram pwolfram added the Ocean label May 1, 2018
@pwolfram pwolfram requested a review from mark-petersen May 1, 2018 19:39
mark-petersen
mark-petersen reviewed May 1, 2018
View reviewed changes
src/framework/mpas_particle_list_types.inc Outdated
@@ -7,7 +7,7 @@
type (mpas_pool_type), pointer :: haloDataPool => NULL()
! pool containing particle data not needing to be passed
! between processors
type (mpas_pool_type), pointer :: nonhaloDataPool => NULL()
!type (mpas_pool_type), pointer :: nonhaloDataPool => NULL()
Copy link
Contributor

@mark-petersen mark-petersen May 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like this is a simple repeated line error? It would need a separate framework PR, but would be easy and fast to get through.

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'm removing the nonHalo feature until it is needed. But, you are right, it is a simple one-liner. I'll remove it and just leave it in framework for now because it isn't a big deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is now removed, so this shouldn't be dependent upon framework.

mark-petersen
mark-petersen approved these changes May 1, 2018
View reviewed changes
Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

@pwolfram I don't have time to test, but don't want to hold this up. Could you describe the testing you've done on this? It's good to compile in both intel and gnu, and compile using debug on one of them to catch unassigned variables. If you test using particles like that, then please merge after removing the framework line.

@mark-petersen
Copy link
Contributor

Whoops - I didn't look carefully enough. I need to make a branch ocean/develop on MPAS-Model, and this PR has to go into that one. Sorry, I'll do it within a day. You'll need to close this PR and make a PR to that one. Only framework PRs go to develop.

@mgduda
Copy link
Contributor

mgduda commented May 1, 2018 •
edited
Loading

I think it is possible to change the target branch of a PR. So after @mark-petersen creates the ocean/develop branch, I think clicking the "Edit" button that the top of this PR will allow the target branch to be set to ocean/develop.

@mark-petersen
Copy link
Contributor

Good point - I see that now, thanks @mgduda

@pwolfram
Copy link
Contributor Author

pwolfram commented May 1, 2018

I think I can just reorient the target branch for the merge. I don't think this needs to be rushed.

@pwolfram pwolfram force-pushed the ocean/LIGHT_restart_cleanup branch from 6658d37 to 129debe Compare May 1, 2018 21:20
@pwolfram
Copy link
Contributor Author

pwolfram commented May 1, 2018

@mark-petersen, can you please let me know when you build the ocean/develop branch?

@mark-petersen
Copy link
Contributor

Done: https://github.com/MPAS-Dev/MPAS-Model/tree/ocean/develop

@pwolfram pwolfram changed the base branch from develop to ocean/develop May 1, 2018 23:34
@pwolfram
Copy link
Contributor Author

pwolfram commented May 1, 2018

Thanks @mark-petersen! I've updated the base branch for the PR.

1 similar comment
@pwolfram
Copy link
Contributor Author

pwolfram commented May 1, 2018

Thanks @mark-petersen! I've updated the base branch for the PR.

@pwolfram
Copy link
Contributor Author

pwolfram commented May 9, 2018

Testing

  • On IC, code compiles with intel and gnu compilers with and without debug on.
  • I've been running with this in E3SM on edison using code from this PR. Compiler used is intel.

@mark-petersen
Copy link
Contributor

Thanks for the testing info. Merge at will.

pwolfram added a commit that referenced this pull request May 9, 2018
@pwolfram
Merge PR #32 'ocean/LIGHT_restart_cleanup' into ocean/develop
57f02a2 
Cleans up LIGHT data structures for improved inter-processor
communication efficiency.
@pwolfram pwolfram closed this May 9, 2018
@pwolfram pwolfram force-pushed the ocean/LIGHT_restart_cleanup branch from 129debe to 57f02a2 Compare May 9, 2018 21:24
@pwolfram pwolfram deleted the ocean/LIGHT_restart_cleanup branch May 9, 2018 21:24
@pwolfram pwolfram added the LIGHT MPAS-O LIGHT tag label May 9, 2019
caozd999 pushed a commit to caozd999/MPAS-Model that referenced this pull request Jan 14, 2021
@xylar
Merge pull request MPAS-Dev#32 from xylar/relative_config_paths
a01ab7a 
Make sure link/copy sources are absolute paths

This allows config files to use relative paths and have them converted to absolute paths 
before links are created or files are copied. This is particularly useful when referring to 
MPAS-Model/ocean/develop or MPAS-Model/landice/develop.
@matthewhoffman matthewhoffman added LIGHT MPAS-O LIGHT tag Ocean labels Mar 17, 2021
syha added a commit to syha/MPAS-Model that referenced this pull request Jul 17, 2022
@syha
Add a missing term for (-qv*d(rho_zz)/dt) in updating the tendency fo…
353b807 
…r theta_m. (MPAS-Dev#32)
climbfuji pushed a commit to climbfuji/MPAS-Model that referenced this pull request Aug 25, 2022
@syha @climbfuji
Add a missing term for (-qv*d(rho_zz)/dt) in updating the tendency fo…
2d43cfd 
…r theta_m. (MPAS-Dev#32)
byoung-joo pushed a commit to byoung-joo/MPAS-Model that referenced this pull request Aug 26, 2022
@syha @byoung-joo
Add a missing term for (-qv*d(rho_zz)/dt) in updating the tendency fo…
4d60b97 
…r theta_m. (MPAS-Dev#32)
LarissaReames pushed a commit to LarissaReames/MPAS-Model that referenced this pull request Jul 8, 2024
@AndersJensen-NOAA
2-moment graupel with predicted graupel density (MPAS-Dev#32)
5d8ed9e 
* Use effective radii from microphysics by default

* Changes for TEMPO submodule

* Support for hail_aware microphysics

* Add if statement to call hail-aware microphysics

* Bug fix for hail-aware flag type

* My python brain forgot the endif
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-petersen mark-petersen mark-petersen approved these changes

Assignees
No one assigned
Labels
LIGHT MPAS-O LIGHT tag Ocean
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pwolfram @mark-petersen @mgduda @matthewhoffman