FMS icon indicating copy to clipboard operation
FMS copied to clipboard

Dm buffer object

Open rem1776 opened this issue 2 years ago • 4 comments

Description adds the file for the buffers with a few of the initial changes to the other files.

Fixes # (issue)

How Has This Been Tested? Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration (e.g. compiler, OS). Include enough information so someone can reproduce your tests.

Checklist:

  • [x] My code follows the style guidelines of this project
  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [x] Any dependent changes have been merged and published in downstream modules
  • [x] New check tests, if applicable, are included
  • [x] make distcheck passes

rem1776 avatar Aug 15 '22 16:08 rem1776

I'm getting a GNU seg fault with the get_buffer assignments 1d and greater:

  class (buffer1d), intent(in) :: this !< 1D buffer object
  class(*), allocatable :: rslt(:)
  if (allocated(this%buffer)) then
    rslt = this%buffer

(fails on the assignment, even though the buffer is allocated and set)

The code is pretty straightforward so i don't know why this is happening, not sure if it's a bug. As a workaround we could allocate copy element by element (not sure how that would affect performace/threading), or return a pointer instead of a copy.

rem1776 avatar Aug 29 '22 17:08 rem1776

The legacy type output_filed_type (see diag_data.F90) seems to have variables buffer, counter, count_0d, and num_elements "together". Should any of these also go into the buffer class?

Probably. Do they belong with the actual buffer or just in the parent class?

thomas-robinson avatar Aug 31 '22 18:08 thomas-robinson

All four are in the same legacy class /type (output_field_type) and they seem to need to go together. The counter and buffer are very similar and updated similarly : they have the same rank; counter is updated (by the math/weighting functions) "usually" when buffer is updated; and updated using the same index extents ( for any i,j,k, sample index) that is applied in the do loops. Count_0d and num_elements are different (different ranks that buffer) but still updated by the same weighting functions. There is more documentation in diag_data.F90.

Overall, right now it seems to me that the buffer class is the most convenient place to keep them even though there is no rank remapping of them. But I suppose there are alternatives.

On Wed, Aug 31, 2022 at 2:52 PM Tom Robinson @.***> wrote:

The legacy type output_filed_type (see diag_data.F90) seems to have variables buffer, counter, count_0d, and num_elements "together". Should any of these also go into the buffer class?

Probably. Do they belong with the actual buffer or just in the parent class?

— Reply to this email directly, view it on GitHub https://github.com/NOAA-GFDL/FMS/pull/1019#issuecomment-1233297052, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKEC3TTOEPAA7XC7YQJ22ULV36SYFANCNFSM56SZWCSA . You are receiving this because you were mentioned.Message ID: @.***>

ngs333 avatar Sep 01 '22 02:09 ngs333

@thomas-robinson It's updated now with the gnu workaround for the get_buffer routines. Now get_buffer is a few subroutines with an intent(out) for the buffer copy, i did some kind of type-bound procedure in the objects so it works like an interface but is called on the object with the output as an arg. I added another variable (typestr) to show the allocated type since i figured that would be needed. still need to update for the docs

@ngs333 @uramirez8707 I added the allocations for the extra members and set them to zero. I allocated the counter to the same shape as buffer, counter_0d to the last dimension size (diurnal axis?), and then num_elements is just the amount of dimensions. Let me know if any needs a change

rem1776 avatar Sep 01 '22 21:09 rem1776