Pass annual-average fields to GLC #1413

billsacks · 2017-04-24T22:42:58Z

These changes are from Mariana Vertenstein (primarily) and myself

Until now, the CESM coupler has been sending CISM daily-average fields,
and then CISM has been averaging these fields to annual-averages in
order to do its mass balance time step using annual-average SMB (and
temperature). This has a number of problems:

(1) We could not restart mid-year, because CISM doesn't write the
necessary partial accumulation information to its restart files.

(2) With Bill Lipscomb's new remapping (bilinear + conservation
correction; coming soon), there were too-large corrections needed
when doing the remapping from CLM to CISM on daily fields.

(3) This led to a discrepancy between TG and BG/IG compsets: In BG/IG,
CISM received the annual average of remapped daily fields; in TG,
CISM received the remapped annual average fields. The remapping is
not a linear operation, so there would typically be differences in
these two runs.

This tag, together with cism2_1_30, allows the coupler to make annual
averages before remapping fields to CISM. This was a bit tricky: CISM
still needs to be called once per day in order to support mid-year
restarts (CESM can only restart at a frequency when all components are
called). So now the GLC calling frequency has been separated from the
GLC averaging frequency. Then GLC/CISM is passed a flag saying whether
the inputs are currently valid; CISM only accumulates averages of the
inputs when they are valid. (In the standard case, this means that CISM
accumulates a single value each year, leaving all of the averaging up to
the coupler. But this supports the ability to still do the averaging in
CISM if this is desired - however, note that mid-year restarts won't
work in that case [CISM now aborts if you try to do a mid-year restart
in that configuration].)

This is implemented in such a way that ACME runs with MPAS-LI should be
unaffected: the default GLC_AVG_PERIOD for ACME is
'glc_coupling_period', which makes things collapse to the old behavior.

There are some mostly-unrelated changes in
src/drivers/mct/cime_config/buildnml and
src/drivers/mct/cime_config/namelist_definition_drv.xml. These use
add_default rather than set_value for the various *_cpl_dt entries,
because in general we should not be calling set_value directly. And,
since we're using add_default, we needed to add
skip_default_entry="true" to the relevant xml variables.

In CESM, this change must be coordinated with cism2_1_30 and
CESM-Development/cime_config#6

Test suite: scripts_regression_tests on yellowstone
Also, aux_glc test suite with cism2_1_30:

out-of-the-box: tests pass, and bit-for-bit except for IG tests
without test_coupling
with GLC_AVG_PERIOD = 'glc_coupling_period': tests pass and
bit-for-bit, except for expected failures in tests that restart
mid-year
Test baseline: For aux_glc tests, baseline was cism2_1_29
Test namelist changes: YES - adds new glc_avg_period
Test status: greater than roundoff-level changes for multi-year runs
with CISM, other than TG

Fixes none

User interface changes?: new xml variable, GLC_AVG_PERIOD

Code review:

cc @whlipscomb @JeremyFyke @stephenprice @matthewhoffman

…ncy than coupling

Mostly changes comments and documentation Also removes a trailing comma from a list in python

I'm not sure this is necessary, but it seems safest

Mariana had changed this to use glcrun_avg_alarm. But I think this work should really be done every time glc is run, even if it's not an averaging time.

If specifying GLC_AVG_PERIOD = 'glc_coupling_period', we'll automatically set the averaging clock to match the glc run clock. This provides more robustness than the earlier hard-coded 'daily' option, in case the glc coupling period is not actually daily.

Abort if it's ever the case that glcrun_avg_alarm is true, but glcrun_alarm is false

In principle we shouldn't need to zero the fields at these times - instead, glc should just ignore the fields at these times. However, some tests (like an ERS or ERI test that stops the final run segment mid-year) can fail if we don't explicitly zero the fields, because the x2g fields can then differ upon restart. The refactored logic in cesm_comp_mod also results in these changes: (1) This code: !---------------------------------------------------- !| cpl -> glc !---------------------------------------------------- if (iamin_CPLALLGLCID .and. glc_prognostic) then call component_exch(glc, flow='x2c', & infodata=infodata, infodata_string='cpl2glc_run', & mpicom_barrier=mpicom_CPLALLGLCID, run_barriers=run_barriers, & timer_barrier='CPL:C2G_BARRIER', timer_comp_exch='CPL:C2G', & timer_map_exch='CPL:c2g_glcx2glcg', timer_infodata_exch='CPL:c2g_infoexch') endif will now only be executed if glcrun_alarm is true. That's what happened on master, but the previous version of the branch executed it even if glcrun_alarm was false. (2) This code: call cesm_comp_barriers(mpicom=mpicom_CPLID, timer='CPL:GLCPREP_BARRIER') call t_drvstartf ('CPL:GLCPREP',cplrun=.true.,barrier=mpicom_CPLID) if (drv_threading) call seq_comm_setnthreads(nthreads_CPLID) and then: if (drv_threading) call seq_comm_setnthreads(nthreads_GLOID) call t_drvstopf ('CPL:GLCPREP',cplrun=.true.) will happen even if .not. glcrun_avg_alarm

…with CISM

jedwards4b · 2017-04-25T14:00:58Z

src/drivers/mct/main/cesm_comp_mod.F90

@@ -2171,6 +2190,8 @@ subroutine cesm_run()
      if (tod            == 0) t24hr_alarm = .true.
      if (month==1 .and. day==1 .and. tod==0) t1yr_alarm = .true.

+      ! TODO(wjs, 2017-04-05) I think glcrun_alarm can be removed from infodata: It used


Can we go ahead and do this as part of the PR?

I have made this change, and am rerunning testing

This used to be used by CLM, but is no longer needed.

apcraig

Looks OK to me.

apcraig · 2017-04-25T16:02:53Z

I'm unclear about this statement "Then GLC/CISM is passed a flag saying whether the inputs are currently valid; CISM only accumulates averages of the inputs when they are valid.". I thought the change was that the coupler was accumulating the input. Should this say, "CISM only runs when the inputs are valid"?

billsacks · 2017-04-25T17:08:07Z

@apcraig - thanks for your review. Sorry, I realized the wording was confusing there because Ieft out a subtlety. I have reworded that paragraph to the following; I hope this makes things more clear:

This tag, together with cism2_1_30, allows the coupler to make annual
averages before remapping fields to CISM. This was a bit tricky: CISM
still needs to be called once per day in order to support mid-year
restarts (CESM can only restart at a frequency when all components are
called). So now the GLC calling frequency has been separated from the
GLC averaging frequency. Then GLC/CISM is passed a flag saying whether
the inputs are currently valid; CISM only accumulates averages of the
inputs when they are valid. (In the standard case, this means that CISM
accumulates a single value each year, leaving all of the averaging up to
the coupler. But this supports the ability to still do the averaging in
CISM if this is desired - however, note that mid-year restarts won't
work in that case [CISM now aborts if you try to do a mid-year restart
in that configuration].)

billsacks · 2017-04-25T18:41:03Z

I have rerun scripts_regression_tests on the latest commit, and everything passes except for the expected failures in K_TestCimeCase

jedwards4b · 2017-04-25T18:49:23Z

There are no expected failures on master - the issues you opened on that have been resolved.
Perhaps you need to rebase?

rljacob · 2017-04-27T01:45:11Z

src/drivers/mct/cime_config/namelist_definition_drv.xml

@@ -1387,7 +1387,7 @@
  <!-- group seq_timemgr_inparm    -->
  <!-- =========================== -->

-  <entry id="atm_cpl_dt" modify_via_xml="ATM_NCPL">
+  <entry id="atm_cpl_dt" modify_via_xml="ATM_NCPL" skip_default_entry="true">


what are these changes for?

@mvertens - can you address this? I don't remember, if I ever knew at all.

rljacob · 2017-04-27T01:51:05Z

src/drivers/mct/shr/seq_timemgr_mod.F90

@@ -1,8 +1,4 @@
 !===============================================================================


Looks like this file had a lot of clean-up in addition to the glc-required changes?

Yes, by @mvertens

mvertens · 2017-04-27T02:32:45Z

Its been so long... Here is my current rationale. If skip_default_entry is not set to "true" then the driver sets all the default values explicitly from the xml file. However, if you have code in the buildnml that explicitly sets these values than you want to ensure that this code is always used - and so you want to have skip_default_entry="true" for those variables.

…

On Wed, Apr 26, 2017 at 7:59 PM, Bill Sacks ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/drivers/mct/cime_config/namelist_definition_drv.xml <#1413 (comment)>: > @@ -1387,7 +1387,7 @@   - <entry id="atm_cpl_dt" modify_via_xml="ATM_NCPL"> + <entry id="atm_cpl_dt" modify_via_xml="ATM_NCPL" skip_default_entry="true"> @mvertens <https://github.com/mvertens> - can you address this? I don't remember, if I ever knew at all. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1413 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlxE93zbMR56AnKY70UeSSSTvYZDoDUks5rz_acgaJpZM4NGzQ8> .

mvertens · 2017-04-27T02:33:46Z

Sorry - hit send too soon. The driver buildnml sets the NCPL values in the following: for comp in comps: ncpl = case.get_value(comp.upper() + '_NCPL') if ncpl is not None: cpl_dt = basedt / int(ncpl) totaldt = cpl_dt * int(ncpl) if totaldt != basedt: expect(False, " %s ncpl doesn't divide base dt evenly" %comp) nmlgen.set_value(comp.lower() + '_cpl_dt', value=cpl_dt) mindt = min(mindt, cpl_dt) On Wed, Apr 26, 2017 at 8:32 PM, Mariana Vertenstein <mvertens@ucar.edu> wrote:

…

Its been so long... Here is my current rationale. If skip_default_entry is not set to "true" then the driver sets all the default values explicitly from the xml file. However, if you have code in the buildnml that explicitly sets these values than you want to ensure that this code is always used - and so you want to have skip_default_entry="true" for those variables. On Wed, Apr 26, 2017 at 7:59 PM, Bill Sacks ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In src/drivers/mct/cime_config/namelist_definition_drv.xml > <#1413 (comment)>: > > > @@ -1387,7 +1387,7 @@ >  >  > > - <entry id="atm_cpl_dt" modify_via_xml="ATM_NCPL"> > + <entry id="atm_cpl_dt" modify_via_xml="ATM_NCPL" skip_default_entry="true"> > > @mvertens <https://github.com/mvertens> - can you address this? I don't > remember, if I ever knew at all. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1413 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AHlxE93zbMR56AnKY70UeSSSTvYZDoDUks5rz_acgaJpZM4NGzQ8> > . >

rljacob · 2017-04-27T02:46:10Z

Why are the skip_default_entry="true" additions needed for the glc feature?

mvertens · 2017-04-27T02:52:40Z

I don't recall. However, its correct to have them there to begin with - since the drv buildnml is setting these values explicitly.

…

On Wed, Apr 26, 2017 at 8:46 PM, Robert Jacob ***@***.***> wrote: Why are the skip_default_entry="true" additions needed for the glc feature? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1413 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlxE3vs6fb3r0d008bV1RSrdH3aROEuks5r0AFzgaJpZM4NGzQ8> .

billsacks · 2017-04-27T03:32:58Z

@mvertens - Sorry it took me so long to get this PR submitted from the time you handed me the code.

I have a feeling that the skip_default_entry="true" changes were needed in conjunction with this change:

--- a/src/drivers/mct/cime_config/buildnml
+++ b/src/drivers/mct/cime_config/buildnml
@@ -101,7 +101,7 @@ def _create_drv_namelists(case, infile, confdir, nmlgen, files):
             totaldt = cpl_dt * int(ncpl)
             if totaldt != basedt:
                 expect(False, " %s ncpl doesn't divide base dt evenly" %comp)
-            nmlgen.set_value(comp.lower() + '_cpl_dt', value=cpl_dt)
+            nmlgen.add_default(comp.lower() + '_cpl_dt', value=cpl_dt)
             mindt = min(mindt, cpl_dt)
 #        elif comp.lower() is not 'cpl':

although I'm not sure that brings us any closer to remembering why these changes were put in on this branch.

mvertens · 2017-04-27T04:52:37Z

@Sacks - thanks for pointing out this diff. The key issue is that if you have add_default - then you MUST have the skip_default_entry="true" - since a call to add_default does not change an already set default value. This is a subtle point that has tripped me up before. And we want to add add_default rather than set_value - so that if needed this can be changed by changing the xml file. Does that make sense?

…

On Wed, Apr 26, 2017 at 9:32 PM, Bill Sacks ***@***.***> wrote: @mvertens <https://github.com/mvertens> - Sorry it took me so long to get this PR submitted from the time you handed me the code. I have a feeling that the skip_default_entry="true" changes were needed in conjunction with this change: --- a/src/drivers/mct/cime_config/buildnml+++ b/src/drivers/mct/cime_config/buildnml@@ -101,7 +101,7 @@ def _create_drv_namelists(case, infile, confdir, nmlgen, files): totaldt = cpl_dt * int(ncpl) if totaldt != basedt: expect(False, " %s ncpl doesn't divide base dt evenly" %comp)- nmlgen.set_value(comp.lower() + '_cpl_dt', value=cpl_dt)+ nmlgen.add_default(comp.lower() + '_cpl_dt', value=cpl_dt) mindt = min(mindt, cpl_dt) # elif comp.lower() is not 'cpl': although I'm not sure that brings us any closer to remembering why these changes were put in on this branch. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1413 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlxE-rZKzJX8bB_M_EiufZCjQkR90f2ks5r0AxqgaJpZM4NGzQ8> .

billsacks · 2017-04-27T11:58:50Z

@mvertens - I just tried changing the ATM_NCPL xml variable and the atm_cpl_dt namelist item in user_nl_cpl, and got the same behavior in master and on this branch: Both allowed me to change ATM_NCPL, and neither allowed me to change atm_cpl_dt in user_nl_cpl.

I know you had a reason for this change: I remember your explaining it to me back in January. I just forget what it was....

matthewhoffman · 2017-04-27T20:05:26Z

src/drivers/mct/main/prep_glc_mod.F90

+
+    !---------------------------------------------------------------
+    ! Description
+    ! Set glc inputs to zero


I realize this is just to generate consistent 'garbage' for times when the field is not valid. One thought would be to fill it with a very obvious garbage value to help protect against the possibility that GLC accidentally tries to actually use it. But I realize just filling with zeroes might be more aesthetically pleasing and such an approach should not be necessary, so feel free to ignore this comment.

matthewhoffman · 2017-04-27T20:09:04Z

@billsacks , a cursory review does not reveal anything that concerns me. Thank you for implementing these changes with ACME considerations in mind. I think this capability will be useful for us as well once we start working on refining our coupling interval.

rljacob · 2017-04-27T20:11:41Z

I also didn't see any showstoppers.

billsacks · 2017-04-27T20:49:08Z

@rljacob and @matthewhoffman - thanks for your reviews of this.

@rljacob - I have added some summarizing information in the top comment, summarizing the changes @mvertens made to the driver namelist generation:

There are some mostly-unrelated changes in
src/drivers/mct/cime_config/buildnml and
src/drivers/mct/cime_config/namelist_definition_drv.xml. These use
add_default rather than set_value for the various *_cpl_dt entries,
because in general we should not be calling set_value directly. And,
since we're using add_default, we needed to add
skip_default_entry="true" to the relevant xml variables.

@matthewhoffman - I see your point about sending garbage, but given the tight timeline here and the fact that you didn't see that as critical, I'm going to leave that as is.

@singhbalwinder

Update CIME to cime5.3 Update ACME to use CIME 5.3.x up to ESMCI hash 6156e0a from Thursday 4/13. scripts_regression_tests and acme_developer tests pass on penn workstation. This CIME change involves a large cosmetic change in CIME. Several directories have been renamed. The main interface to CIME, the scripts dir remains unchanged. cime/cime_config is renamed to cime/config The Fortran source code has been consolidated in src directory: externals, components, share, and drivers The driver_cpl directory has been moved to src/drivers/mct to prepare for multiple coupler options The share/csm_share/share directory has been renamed src/share/util. The cime script infrastructure source, utils/python/CIME, has moved to scripts/lib/CIME. @singhbalwinder : please make sure PNNL cluster merge look correct @mfdeakin-sandia : please look at cprnc/CMakeLists.txt and also confirm no run_acme changes. Fixes #1311 Fixes #1380 Fixes #1382 Fixes #1383 Fixes #1396 Fixes #1402 Fixes #1416 [BFB] * agsalin/update-to-cime5.3: (5649 commits) Fix titan problem Reset invalid pio_numiotasks Fix merge mistake Re-add eca testmod, was somehow lost in big merge Restore ACME version of shr_orb_cosz Fixed whitespace errors in Makefile killing COSP HOMME test typo fix Fixes to get tests to pass. Update ACME for new CIME directory structure More renaming. Rename ACME/cime directories in prep for CIME merge HOMME Improvement More edits to README Suggest a more portable usage of mktemp Tweak a comment Add comment on acme side Fix case for new unit_testing attribute to get_mpirun Allow run_tests.py to auto-determine the machine name fix an error make user-compset a seperate test ...

@singhbalwinder

Update CIME to cime5.3 Update ACME to use CIME 5.3.x up to ESMCI hash 6156e0a from Thursday 4/13. scripts_regression_tests and acme_developer tests pass on penn workstation. This CIME change involves a large cosmetic change in CIME. Several directories have been renamed. The main interface to CIME, the scripts dir remains unchanged. cime/cime_config is renamed to cime/config The Fortran source code has been consolidated in src directory: externals, components, share, and drivers The driver_cpl directory has been moved to src/drivers/mct to prepare for multiple coupler options The share/csm_share/share directory has been renamed src/share/util. The cime script infrastructure source, utils/python/CIME, has moved to scripts/lib/CIME. @singhbalwinder : please make sure PNNL cluster merge look correct @mfdeakin-sandia : please look at cprnc/CMakeLists.txt and also confirm no run_acme changes. Fixes #1311 Fixes #1380 Fixes #1382 Fixes #1383 Fixes #1396 Fixes #1402 Fixes #1416 [BFB] * agsalin/update-to-cime5.3: (5649 commits) Fix titan problem Reset invalid pio_numiotasks Fix merge mistake Re-add eca testmod, was somehow lost in big merge Restore ACME version of shr_orb_cosz Fixed whitespace errors in Makefile killing COSP HOMME test typo fix Fixes to get tests to pass. Update ACME for new CIME directory structure More renaming. Rename ACME/cime directories in prep for CIME merge HOMME Improvement More edits to README Suggest a more portable usage of mktemp Tweak a comment Add comment on acme side Fix case for new unit_testing attribute to get_mpirun Allow run_tests.py to auto-determine the machine name fix an error make user-compset a seperate test ...

@singhbalwinder

Update CIME to cime5.3 Update ACME to use CIME 5.3.x up to ESMCI hash 6156e0a from Thursday 4/13. scripts_regression_tests and acme_developer tests pass on penn workstation. This CIME change involves a large cosmetic change in CIME. Several directories have been renamed. The main interface to CIME, the scripts dir remains unchanged. cime/cime_config is renamed to cime/config The Fortran source code has been consolidated in src directory: externals, components, share, and drivers The driver_cpl directory has been moved to src/drivers/mct to prepare for multiple coupler options The share/csm_share/share directory has been renamed src/share/util. The cime script infrastructure source, utils/python/CIME, has moved to scripts/lib/CIME. @singhbalwinder : please make sure PNNL cluster merge look correct @mfdeakin-sandia : please look at cprnc/CMakeLists.txt and also confirm no run_acme changes. Fixes #1311 Fixes #1380 Fixes #1382 Fixes #1383 Fixes #1396 Fixes #1402 Fixes #1416 [BFB] * agsalin/update-to-cime5.3: (5649 commits) Fix titan problem Reset invalid pio_numiotasks Fix merge mistake Re-add eca testmod, was somehow lost in big merge Restore ACME version of shr_orb_cosz Fixed whitespace errors in Makefile killing COSP HOMME test typo fix Fixes to get tests to pass. Update ACME for new CIME directory structure More renaming. Rename ACME/cime directories in prep for CIME merge HOMME Improvement More edits to README Suggest a more portable usage of mktemp Tweak a comment Add comment on acme side Fix case for new unit_testing attribute to get_mpirun Allow run_tests.py to auto-determine the machine name fix an error make user-compset a seperate test ...

Mariana Vertenstein and others added 13 commits April 5, 2017 21:12

first set of commits for doing glc time averaging at different freque…

380f22f

…ncy than coupling

changes to get glc to time average inside the coupler over a year

f168784

changes to have new yearly averaging for cism

77327f8

bug fix for compilation

12b11ff

Minor cleanup

8878fd7

Mostly changes comments and documentation Also removes a trailing comma from a list in python

Call seq_infodata_putData for glc_valid_input from all tasks

40059b8

I'm not sure this is necessary, but it seems safest

Do GLC RECV-POST based on glcrun_alarm

076847b

Mariana had changed this to use glcrun_avg_alarm. But I think this work should really be done every time glc is run, even if it's not an averaging time.

Add an error check

97068a5

Abort if it's ever the case that glcrun_avg_alarm is true, but glcrun_alarm is false

Modify some documentation of required settings for mid-year restarts …

8ce40ab

…with CISM

Merge branch 'master' into billsacks/glc_tavg_v2

fea3f6b

Fix merge problem

cc6d449

billsacks added the tp: drv mct label Apr 24, 2017

billsacks self-assigned this Apr 24, 2017

billsacks requested review from rljacob and apcraig April 24, 2017 22:42

billsacks added the in progress label Apr 24, 2017

jedwards4b approved these changes Apr 25, 2017

View reviewed changes

Remove glcrun_alarm from seq_infodata

f7f0ff6

This used to be used by CLM, but is no longer needed.

apcraig approved these changes Apr 25, 2017

View reviewed changes

rljacob reviewed Apr 27, 2017

View reviewed changes

matthewhoffman reviewed Apr 27, 2017

View reviewed changes

rljacob approved these changes Apr 27, 2017

View reviewed changes

billsacks changed the title ~~DO NOT MERGE YET: Pass annual-average fields to GLC~~ Pass annual-average fields to GLC Apr 27, 2017

billsacks merged commit fe82bd6 into ESMCI:master Apr 27, 2017

billsacks removed the in progress label Apr 27, 2017

billsacks deleted the billsacks/glc_tavg_v2 branch April 27, 2017 20:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass annual-average fields to GLC #1413

Pass annual-average fields to GLC #1413

billsacks commented Apr 24, 2017 •

edited

Loading

jedwards4b Apr 25, 2017

billsacks Apr 25, 2017

apcraig left a comment

apcraig commented Apr 25, 2017

billsacks commented Apr 25, 2017

billsacks commented Apr 25, 2017

jedwards4b commented Apr 25, 2017

rljacob Apr 27, 2017

billsacks Apr 27, 2017

rljacob Apr 27, 2017

billsacks Apr 27, 2017

mvertens commented Apr 27, 2017 via email

mvertens commented Apr 27, 2017 via email

rljacob commented Apr 27, 2017

mvertens commented Apr 27, 2017 via email

billsacks commented Apr 27, 2017

mvertens commented Apr 27, 2017 via email

billsacks commented Apr 27, 2017

matthewhoffman Apr 27, 2017

matthewhoffman commented Apr 27, 2017

rljacob commented Apr 27, 2017

billsacks commented Apr 27, 2017

		@@ -1,8 +1,4 @@
		!===============================================================================

Pass annual-average fields to GLC #1413

Pass annual-average fields to GLC #1413

Conversation

billsacks commented Apr 24, 2017 • edited Loading

jedwards4b Apr 25, 2017

Choose a reason for hiding this comment

billsacks Apr 25, 2017

Choose a reason for hiding this comment

apcraig left a comment

Choose a reason for hiding this comment

apcraig commented Apr 25, 2017

billsacks commented Apr 25, 2017

billsacks commented Apr 25, 2017

jedwards4b commented Apr 25, 2017

rljacob Apr 27, 2017

Choose a reason for hiding this comment

billsacks Apr 27, 2017

Choose a reason for hiding this comment

rljacob Apr 27, 2017

Choose a reason for hiding this comment

billsacks Apr 27, 2017

Choose a reason for hiding this comment

mvertens commented Apr 27, 2017 via email

mvertens commented Apr 27, 2017 via email

rljacob commented Apr 27, 2017

mvertens commented Apr 27, 2017 via email

billsacks commented Apr 27, 2017

mvertens commented Apr 27, 2017 via email

billsacks commented Apr 27, 2017

matthewhoffman Apr 27, 2017

Choose a reason for hiding this comment

matthewhoffman commented Apr 27, 2017

rljacob commented Apr 27, 2017

billsacks commented Apr 27, 2017

billsacks commented Apr 24, 2017 •

edited

Loading