Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@MiCurry
Copy link
Contributor

@MiCurry MiCurry commented Jan 20, 2020

This commit adds a new module to the MPAS init_atmosphere core, mpas_stack, which is a simple and extensible stack implementation. It uses Fortran 2003's polymorphic features to allow different 'payloads' to be associated with a stack; thus it can be used for a number of applications.

At current, this module is not being used by any part of the init_atmosphere core, but a future PR could make use of its features.

A copy of mpas_init_atm_core.F, that uses and contains tests for mpas_stack.F, has been uploaded as a gist, https://gist.github.com/MiCurry/22ad2085f0ba10e325d96449a97a8434, which can be used to test mpas_stack.F.

@MiCurry
Copy link
Contributor Author

MiCurry commented Jan 20, 2020

A companion repository has also been setup, https://github.com/MiCurry/mpas_stack to help test for memory leaks.

@MiCurry MiCurry closed this Jan 20, 2020
@MiCurry MiCurry reopened this Jan 21, 2020
@MiCurry MiCurry force-pushed the init_atmosphere/mpas_stack branch from 15b504b to 8b5cbb1 Compare January 21, 2020 16:44
@MiCurry
Copy link
Contributor Author

MiCurry commented Jan 21, 2020

@mgduda I think this is good to go, at least for inspection. I've compiled with Intel, GNU and PGI, and tested with Intel and GNU. I'm working on testing with PGI.

There is a single pitfall when using polymorphic types and the Intel compiler (which I think we've ran into and discussed before). With GNU, you can define your item as a type (i.e. type(cell_t) :: cell) and successfully push and deallocate this item with mpas_stack_free, but with Intel you cannot. You're able to allocate and push the item just fine, but when you try to deallocate the item you get the runtime error:

forrtl: severe (173): A pointer passed to DEALLOCATE points to an object that cannot be deallocated

Which appears to be a bug. The workaround (as Doctor Fortran posted), is to use class instead of type (i.e class(cell_t) :: cell). For our usage, I don't think it will cause any problems.

src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
@mgduda
Copy link
Contributor

mgduda commented Jan 24, 2020

@MiCurry Functionally, I think the module looks good. Besides a bit of clean-up, I'd just like to push for a renaming of the derived types.

@MiCurry MiCurry force-pushed the init_atmosphere/mpas_stack branch 2 times, most recently from 3f98327 to 3896194 Compare January 27, 2020 17:59
@MiCurry
Copy link
Contributor Author

MiCurry commented Jan 27, 2020

Okay, @mgduda, I've made those changes that you requested in terms of clean-up and renamed the derived types as we discussed above! So I think this PR is good to go!

src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
src/core_init_atmosphere/mpas_stack.F Outdated Show resolved Hide resolved
This commit adds a new module to the MPAS init_atmosphere core, mpas_stack,
which is a simple stack implementation. At current, this module is not being
used by any part of the init_atmosphere core.
@MiCurry MiCurry force-pushed the init_atmosphere/mpas_stack branch from 3896194 to 354c5c3 Compare January 29, 2020 19:04
@MiCurry
Copy link
Contributor Author

MiCurry commented Jan 29, 2020

@mgduda Okay, those changes have been made!

@mgduda mgduda self-requested a review January 30, 2020 20:58
Copy link
Contributor

@mgduda mgduda left a comment

Choose a reason for hiding this comment

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

@MiCurry Thanks for making the requested changes! This looks good.

mgduda added a commit that referenced this pull request Jan 30, 2020
This merge adds a new module to the MPAS init_atmosphere core, mpas_stack, which
is a simple and extensible stack implementation. It uses Fortran 2003
polymorphism to allow different 'payloads' to be pushed and popped on a stack;
thus it can be used for a number of applications.

At present, this module is not being used by any part of the init_atmosphere
core, but future code could make use of its features.

* init_atmosphere/mpas_stack:
  Add mpas_stack to init_atmosphere core
@mgduda mgduda merged commit 354c5c3 into MPAS-Dev:develop Jan 30, 2020
@MiCurry MiCurry deleted the init_atmosphere/mpas_stack branch July 27, 2020 16:55
jonbob added a commit to E3SM-Project/E3SM that referenced this pull request Jul 27, 2020
#3737)

Update MPAS framework

This PR brings in a new mpas-source submodule with updates to the mpas framework
itself, plus changes to the cores supporting the framework update. Some changes
are made to the atmosphere core, even though it is not used by E3SM, in order to
maintain consistency with the framework. This update includes the following
individual branches and PRs, many of which are additions to the makefiles for
summit, or optional libraries:
* az/azamat/mpas-cmake/mpas-tool-dir  (MPAS-Dev/MPAS-Model#629)
* init_atmosphere/kd_tree_ties  (MPAS-Dev/MPAS-Model#630)
* mark-petersen/framework/add_FillValue  (MPAS-Dev/MPAS-Model#616)
* mark-petersen/framework/add_lapack_option  (MPAS-Dev/MPAS-Model#613)
* amametjanov/framework/nullify-field-pointers  (MPAS-Dev/MPAS-Model#578)
* framework/makefile_e3sm  (MPAS-Dev/MPAS-Model#603)
* xylar/framework/make_shell_bash  (MPAS-Dev/MPAS-Model#594)
* registry/missing_value  (MPAS-Dev/MPAS-Model#562)
* pwolfram/updates_make_intel_stack  (MPAS-Dev/MPAS-Model#592)
* azamat/framework/pgi-cpr-omp-workaround  (MPAS-Dev/MPAS-Model#449)
* az/framework/e3sm-cmake-qnosmp-typo  (MPAS-Dev/MPAS-Model#579)
* xylar/compass/add_prerequisite_tag
* atmosphere/fix_timekeeping_imports (MPAS-Dev/MPAS-Model#582)
* xylar/fix_docs_ci  (MPAS-Dev/MPAS-Model#575)
* xylar/add_compass_docs  (MPAS-Dev/MPAS-Model#472)
* philipwjones/framework/summitmake  (MPAS-Dev/MPAS-Model#536)
* atmosphere/atm_core_cleanup  (MPAS-Dev/MPAS-Model#548)
* init_atmosphere/parse_geoindex  (MPAS-Dev/MPAS-Model#459)
* xylar/compass/add_copy_file  (MPAS-Dev/MPAS-Model#467)
* init_atmosphere/kd_tree  (MPAS-Dev/MPAS-Model#438)
* init_atmosphere/mpas_stack  (MPAS-Dev/MPAS-Model#426)
* operators/add_mpas_in_cell  (MPAS-Dev/MPAS-Model#400)

Fixes #3396
Fixes #3236

[BFB]
jonbob added a commit to E3SM-Project/E3SM that referenced this pull request Jul 29, 2020
Update MPAS framework

This PR brings in a new mpas-source submodule with updates to the mpas framework
itself, plus changes to the cores supporting the framework update. Some changes
are made to the atmosphere core, even though it is not used by E3SM, in order to
maintain consistency with the framework. This update includes the following
individual branches and PRs, many of which are additions to the makefiles for
summit, or optional libraries:
* az/azamat/mpas-cmake/mpas-tool-dir  (MPAS-Dev/MPAS-Model#629)
* init_atmosphere/kd_tree_ties  (MPAS-Dev/MPAS-Model#630)
* mark-petersen/framework/add_FillValue  (MPAS-Dev/MPAS-Model#616)
* mark-petersen/framework/add_lapack_option  (MPAS-Dev/MPAS-Model#613)
* amametjanov/framework/nullify-field-pointers  (MPAS-Dev/MPAS-Model#578)
* framework/makefile_e3sm  (MPAS-Dev/MPAS-Model#603)
* xylar/framework/make_shell_bash  (MPAS-Dev/MPAS-Model#594)
* registry/missing_value  (MPAS-Dev/MPAS-Model#562)
* pwolfram/updates_make_intel_stack  (MPAS-Dev/MPAS-Model#592)
* azamat/framework/pgi-cpr-omp-workaround  (MPAS-Dev/MPAS-Model#449)
* az/framework/e3sm-cmake-qnosmp-typo  (MPAS-Dev/MPAS-Model#579)
* xylar/compass/add_prerequisite_tag
* atmosphere/fix_timekeeping_imports (MPAS-Dev/MPAS-Model#582)
* xylar/fix_docs_ci  (MPAS-Dev/MPAS-Model#575)
* xylar/add_compass_docs  (MPAS-Dev/MPAS-Model#472)
* philipwjones/framework/summitmake  (MPAS-Dev/MPAS-Model#536)
* atmosphere/atm_core_cleanup  (MPAS-Dev/MPAS-Model#548)
* init_atmosphere/parse_geoindex  (MPAS-Dev/MPAS-Model#459)
* xylar/compass/add_copy_file  (MPAS-Dev/MPAS-Model#467)
* init_atmosphere/kd_tree  (MPAS-Dev/MPAS-Model#438)
* init_atmosphere/mpas_stack  (MPAS-Dev/MPAS-Model#426)
* operators/add_mpas_in_cell  (MPAS-Dev/MPAS-Model#400)

Fixes #3396
Fixes #3236

[BFB]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.