cohort_index could be improved

[rgknox]

Summary of Issue:

Every machine node and all of its threads share a global variable called udata%cohort_index. This index is incremented whenever a new cohort comes into existence, and the new value is pushed onto the cohort's value, where currentcohort%cohort_index = udata%cohort_index. This variable is not written to restarts, and from what I can tell, udata%cohort_index is set to 0 at the start of all runs.

The cohort index has two purposes:

Purpose 1: the index prevents cohorts from self fusion when two cohort lists are crossed

Purpose 2: the index is useful for diagnostics while tracking cohort behavior from one routine to the other

I see three issues with the current implementation, I consider them low-priority.

Issue 1: It is remotely possible that two threads on the same node could increment udata%cohort_index before each thread pushes that value onto the cohort vector. But... who cares right? The index is really only for diagnostic purposes, so what if two cohorts on different gridcells have the same index, we would never know. Also, we are not writing these to restarts, so this type of issues shouldn't affect bit4bit regression testing.

Issue 2: A 4-byte integer can only store numbers up to 2 billion or so on the positive side. So it may be possible on some massively threaded simulation that is running some incredibly long amount of time that the number could exceed 2 billion... but I doubt it.

Issue 3: Cohort variables are memory hungry, if we don't need them we should remove them when possible.

What is the changeset ID of the code: 0471ef9

Possible solutions:

1. change the cohort index to an integer or character string that reflects its creation date, patch number cohort order number and other potentially relevant information.

2. just remove it, it takes up space and it is not necessary for preventing self fusion ( the associated intrinsic accepts 2 arguments to test if two pointers point to the same memory space).

[rosiealice]

While my gut reaction is to simplify things and go with #2, I have
occasionally found this to be useful for debugging purposes, for being sure
that something happening to cohort'n' in one subroutine is the same cohort
in another, that sort of thing.

On 18 May 2016 at 15:14, Ryan Knox [email protected] wrote:

Summary of Issue:

Every machine node and all of its threads share a global variable called
udata%cohort_index. This index is incremented whenever a new cohort comes
into existence, and the new value is pushed onto the cohort's value, where
currentcohort%cohort_index = udata%cohort_index. This variable is not
written to restarts, and from what I can tell, udata%cohort_index is set to
0 at the start of all runs.

The cohort index has two purposes:

Purpose 1: the index prevents cohorts from self fusion when two cohort
lists are crossed

Purpose 2: the index is useful for diagnostics while tracking cohort
behavior from one routine to the other

I see three issues with the current implementation, I consider them
low-priority.

Issue 1: It is remotely possible that two threads on the same node could
increment udata%cohort_index before each thread pushes that value onto the
cohort vector. But... who cares right? The index is really only for
diagnostic purposes, so what if two cohorts on different gridcells have the
same index, we would never know. Also, we are not writing these to
restarts, so this type of issues shouldn't affect bit4bit regression
testing.

Issue 2: A 4-byte integer can only store numbers up to 2 billion or so on
the positive side. So it may be possible on some massively threaded
simulation that is running some incredibly long amount of time that the
number could exceed 2 billion... but I doubt it.

Issue 3: Cohort variables are memory hungry, if we don't need them we
should remove them when possible.
What is the changeset ID of the code: 0471ef9
0471ef9 Possible
solutions:

1. change the cohort index to an integer or character string that reflects
   its creation date, patch number cohort order number and other potentially
   relevant information.

2. just remove it, it takes up space and it is not necessary for
   preventing self fusion ( the associated intrinsic accepts 2 arguments to
   test if two pointers point to the same memory space).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#60

---

Dr Rosie A. Fisher

Terrestrial Sciences Section
Climate and Global Dynamics
National Center for Atmospheric Research
1850 Table Mesa Drive
Boulder, Colorado, 80305
USA.
+1 303-497-1706

http://www.cgd.ucar.edu/staff/rfisher/

[bandre-ucar]

Issue 1: It is remotely possible that two threads on the same node could increment udata%cohort_index before each thread pushes that value onto the cohort vector. But... who cares right? The index is really only for diagnostic purposes

@rgknox : I'm not sure what your experience has been, but I've found people tend to get a bit grumpy if they are trying to debug a problem and get inaccurate diagnostic information because of an unrelated problem. Especially when it is difficult to reproduce behavior related to a race condition.

[rosiealice]

Perhaps we just need information on the cohort identity within a single
patch, given that each patch has its own identifier already. Would that
solve the threading issue?

On 19 May 2016 at 09:32, Ben Andre [email protected] wrote:

Issue 1: It is remotely possible that two threads on the same node could
increment udata%cohort_index before each thread pushes that value onto the
cohort vector. But... who cares right? The index is really only for
diagnostic purposes

@rgknox https://github.com/rgknox : I'm not sure what your experience
has been, but I've found people tend to get a bit grumpy if they are trying
to debug a problem and get inaccurate diagnostic information because of an
unrelated problem. Especially when it is difficult to reproduce behavior
related to a race condition.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#60 (comment)

---

Dr Rosie A. Fisher

Terrestrial Sciences Section
Climate and Global Dynamics
National Center for Atmospheric Research
1850 Table Mesa Drive
Boulder, Colorado, 80305
USA.
+1 303-497-1706

http://www.cgd.ucar.edu/staff/rfisher/

[rgknox]

@bandre-ucar: Its certainly not ideal, but I'm weighing the pro/cons and opportunity cost here. Maybe this diagnostic is barely used. And, since the threads are split at the grid level and not the cohort level, the duplicate id's will be discernible (with extra effort).

[rgknox]

cohort_index has been deprecated and is no longer used, closing