Update ACS BLEVCORR routines to accommodate new supported subarrays #312

mdlpstsci · 2018-05-29T18:14:28Z

Addresses IT #168 (EDIT: Fixed linking)

The new supported subarrays (circa mid-2016) need to have the improved bias level processing applied during CALACS processing akin to what is done for the full frame data. In detail only the bias shift correction is applicable to subarray data. This is currently a work in progress in that only the new WFC 2K subarrays are handled. The WFC 512 and 1K subarrays do not have virtual overscan, so there is no "magic square" data. This issue will be addressed when an alternative to the "magic square" is defined. Also, the polarizer and ramp subarrays are not yet handled.

It should be noted the block of code handling the logic of which BLEV algorithm (original or new and improved) should be applied and the data appropriate for each algorithm has been re-written for clarity and at the request of the team lead. While it is more verbose, the logic is cleaner.

pllim

Some comments provided. I cannot approve or otherwise, as I don't think I am in the position to do so. As long as regression tests pass, should be okay? You might also need to add new test cases.

pllim · 2018-05-29T18:44:03Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+ *
+ * 16-May-2018 M.D. De La Pena: Generalized bias_shift_corr() to handle subarray data.
+ */
+int bias_shift_corr(ACSInfo *acs, int nGroups, ...) {


Wow, this is magical! 🔮

...So much easier and more obvious with an OO language.

pllim · 2018-05-29T18:53:41Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+  /* For full frame data, there are four amps in use.  For supported subarray
+   * data, there is only one amp in use.  The actual amp in use must be determined. 
+   */
+  int numAmpsInUse = (acs->subarray == YES) ? 1 : NAMPS;


Why is numAmpsInUse = NAMPS wrong for subarray cases? What about subarrays with 2 amps? For example, AD or BC. Will those never get to this point in the pipeline?

NAMPS is hard-coded to 4 which nicely accommodates full frame data. However, the new, "supported" WFC subarrays are single amp (512, 1K, or 2K). This may have to be updated once the POL and RAMP data are incorporated into this new algorithm. All other WFC subarrays will use the old blev algorithm.

This may have been worth hiding within a func, in case the logic becomes more complicated in the future.

pllim · 2018-05-29T19:02:54Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

-      for (k = 13; k <= 22; k++) {
-        sum += ampdata[arr_cols*j + k];
-        num++;
+      for (j = 2057; j <= 2066; j++) {


Even prior to this PR, I was uncomfortable with these magic numbers. Perhaps it is time to make them constant variables with meaningful names?

This comment also applies to other occurrences of magic numbers in this file.

This is already in progress. An updated overscan table has been created with primary header keywords and a new binary table in the second extension to accommodate amp-dependent values.

pllim · 2018-05-29T19:35:16Z

pkg/acs/calacs/acsccd/doccd.c

@@ -32,6 +35,8 @@ static void dqiMsg (ACSInfo *, int);
 static void BiasMsg (ACSInfo *, int);
 static void BlevMsg (ACSInfo *, int);
 static void SinkMsg (ACSInfo *, int);
+/* These are the subarray supported apertures for the bias shift correction */
+static const char *subApertures[] = {"WFC1A-2K", "WFC1B-2K", "WFC2C-2K", "WFC2D-2K"};


If you ever want to add polarizers, keep in mind that the aperture name never changed after flight software update, so same POL aperture is "old subarray" before that update, and "new subarray" after. Becareful with those.

Understood. The POL and RAMP data have to be checked for actual size and also possibly date as the discriminants.

pllim · 2018-05-29T19:53:03Z

pkg/acs/calacs/acsccd/doccd.c

-                return (status);
+       ***FIX MDD: What about polarizer and ramp apertures?
+    */
+    Bool supportedSubAperture = NO;


I feel that this should be inside the logic for WFC only, because this same code is also used to process HRC.

FYI: I've been trying to move stuff over to using stdbool.h::bool instead.

pllim · 2018-05-29T19:59:29Z

pkg/acs/calacs/acsccd/doccd.c

+
+       } /* End of only WFC detector */
+
+       /* All HRC and SBC data - use the original blev correction */


SBC data don't get processed here. This is only for CCD, not MAMA. (Please correct me if I misremember.)

Ooops. I got a bit too inclusive in my comments - here there be no SBC.

mdlpstsci · 2018-08-01T20:50:41Z

Polarized and ramp data are now handled by this upgrade. The new WFC 512 and 1K subarrays are not yet accommodated by this upgrade as they do not have virtual overscan, so there is no "magic square" data to use. The team needs to determine an alternative to the "magic square". The additional work to handle the WFC 512 and 1K subarrays will be handled by a new issue.

mdlpstsci · 2018-08-07T21:08:47Z

Nate (ACS Team) sent on 07 August 2018:
"TL;DR: I approve the updates for the bias shift as they properly handle full-frame Saturn data (Proposal ID: 12395) that was forced into the correct subarray format for the new bias-shift algorithm to be applied in BLEVCORR."

mdlpstsci · 2018-08-07T21:15:18Z

@pllim Can you please re-review? Basically, I added a lot of code to (1) handle all the cases for the full frame and subarray data with regard to the bias shift correction, and (2) free memory. I will note the logic for application of the bias shift correction has been expanded at the request of the ACS Team lead. The code is now rather verbose and repetitive, but it is more approachable to an astronomer.

jamienoss · 2018-08-08T14:11:05Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+
+  /* Initializing arguments to store all the values being passed to this routine via "..." */
+  va_start (arguments, nGroups);
+  {


Why the extra scope?

Perhaps you want to scope arguments, which is a very good idea, in which case this needs to be moved up just above va_list arguments; and the closing, just after va_end (arguments);.

If that was not your intention, I would scope the va_list.

jamienoss · 2018-08-08T14:17:06Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+   * the physical overscan and must be divided by 2.  If this is a subarray, then "nx" represents
+   * just the columns for a single amp plus physical overscan.
+   */
+  const int arr_rows = sg[0]->sci.data.ny;


sg[0] is being accessed without first being checked. This can be done here, or better still when first assigned to sg so as to shrink it to only valid ptrs (bookkeeping required of nGroups etc).

jamienoss · 2018-08-08T14:17:38Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+       /* sg[0] is chip2 and sg[1] is chip 1 for a full frame image */
+       {unsigned int i;
+       for (i = 0; i < nGroups; i++) {
+           sg[i]= va_arg (arguments, SingleGroup *);


You may want to check that they're not NULL here, see comment below.

jamienoss · 2018-08-08T14:18:28Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+ *
+ * 16-May-2018 M.D. De La Pena: Generalized bias_shift_corr() to handle subarray data.
+ */
+int bias_shift_corr(ACSInfo *acs, int nGroups, ...) {


Add assert(acs);.

Also:

if (nGroups < 1) return HSTCAL_OK; // or NOTHING_TO_DO, NO_GOOD_DATA, INTERNAL_ERROR

Actually, if going for INTERNAL_ERROR I would instead assert. This is a much better way to handle internal errors as it gives the src line in question.

I tend to treat this kind of thing either as a NOP and would return HSTCAL_OK or assert, upto you to think about the function's semantics.

jamienoss · 2018-08-08T14:43:06Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+   * just the columns for a single amp plus physical overscan.
+   */
+  const int arr_rows = sg[0]->sci.data.ny;
+  const int arr_cols = (acs->subarray == NO) ? sg[0]->sci.data.nx/2 : sg[0]->sci.data.nx;


Rename arr_cols more semantically specific, e.g. ampArrCols etc.

FYI: (I know the mix already existed) but I don't like the mix of camel case and underscores.

Keeping the arr_cols vs ampArrCols as the remaining functions in this file also use this variable name for a total of 87 name changes. I want to keep the functions consistent in their terminology.

jamienoss · 2018-08-08T14:48:13Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+   * just the columns for a single amp plus physical overscan.
+   */
+  const int arr_rows = sg[0]->sci.data.ny;
+  const int arr_cols = (acs->subarray == NO) ? sg[0]->sci.data.nx/2 : sg[0]->sci.data.nx;


Regarding the logic: I'm only now getting the impression that the singleGroups being passed in are actually amps. Is this correct? So what does this func act on amps or...? If amps, then nx should already be correct, right? Then why the /2?

jamienoss · 2018-08-08T14:51:10Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

  /* arrays below are in order of amp A, B, C, D */

-  /* time constant of AC high pass filter in external pre-amp. */
+  /* time constant of AC high pass filter in external pre-amp in microseconds */
  const double time_const[NAMPS] = {37.290251, 36.180001, 37.867770, 42.461249};


If in microsecs, reflect this in the param name, e.g. usecTimeConst.

jamienoss · 2018-08-08T14:58:43Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+  /* array for amp data and amp data + gap pixels */
+  double *ampdata = malloc(arr_rows * arr_cols * sizeof(double));
+  addPtr(&ptrReg, ampdata, &free);
+  double *ampdata_gap = malloc(nquad_pix * sizeof(double));


All of the above mallocs need to be NULL checked handling OUT_OF_MEMORY errors.

Indeed. Went to great trouble to add these checks in other routines, but I neglected to add the checks here.

jamienoss · 2018-08-08T14:59:14Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

-
-  /* allocate space for true DC bias levels array */
+
+  PtrRegister ptrReg;


jamienoss · 2018-08-08T15:06:22Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+   */
+  int ampInUse = 0;
+  {unsigned int i;
+  for (i = 0; i < numAmpsInUse; i++) {


This should be a function and I believe it already is, parseWFCamps() (src) and should be used in conjunction with AMPSORDER (src).

jamienoss · 2018-08-08T16:29:54Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

-        ampdata[arr_cols*j + k] = ampdata_gap[(arr_cols + ngap_pix)*j + k];
-      }
-    }
+      /* copy corrected data back to ampdata */


Is it not possible to combine this op within the loop above (correction comp) and assign the correction directly to ampdata? This would be more efficient.

jamienoss · 2018-08-08T16:34:19Z

pkg/acs/calacs/acsccd/findover.c


    int SameInt (int, int);
    int SameString (char *, char *);

+    int foundit = NO;
+    *overscan = NO;


Good job moving this to before the return.

jamienoss · 2018-08-08T16:37:51Z

pkg/acs/calacs/lib/getacskeys.c

-    return (status);
+       return (status);
+    {unsigned int i;
+       for (i=0; i < strlen(acs->aperture); i++) {


indentation

jamienoss · 2018-08-08T16:42:08Z

pkg/acs/calacs/lib/getacskeys.c


 	if (GetKeyStr (phdr, "APERTURE", USE_DEFAULT, "", acs->aperture, ACS_CBUF))
-    return (status);
+       return (status);
+    {unsigned int i;


This should be a func toUpper() that excepts a string. E.g. use this utility in cfitsio Convert a character string to uppercase (operates in place). void fits_uppercase / ffupch (char *string).

jamienoss · 2018-08-08T16:48:33Z

pkg/acs/calacs/lib/getacskeys.c

+       return (status);
+    {unsigned int i;
+       for (i=0; i < strlen(acs->aperture); i++) {
+           acs->aperture[i] = toupper (acs->aperture[i]);


This is only acceptable/valid if it is "illegal" for the char set to be [a-z], if not it must be removed. Even if it is I really don't think it is this function's responsibility to do so.

Imagine someone wanting to write a validity wrapper to GetACSKeys(), they now can't because the functionality/semantics has been mangled.

jamienoss · 2018-08-08T16:54:52Z

pkg/acs/calacs/acsccd/doccd.c


    int to_electrons(ACSInfo *, SingleGroup *);
    int doBias (ACSInfo *, SingleGroup *);
    int biasHistory (ACSInfo *, Hdr *);
    int doBlev (ACSInfo *, SingleGroup *, int, float *, int *, int *);
-    int bias_shift_corr(ACSInfo *, SingleGroup *, SingleGroup *);
+    int bias_shift_corr(ACSInfo *, int, ...);


This should be in a header. It should also have a comment describing the variadic arg.

jamienoss · 2018-08-08T16:57:07Z

pkg/acs/calacs/acsccd/doccd.c

    /* set up an array of SingleGroups to hold all image extensions */
+    SingleGroup * x = NULL;   /* used for both input and output */
    x = (SingleGroup *) malloc(acs_info->nimsets * sizeof(SingleGroup));


Would have been good to have taken the opportunity and removed the cast.

and also changed sizeof(SingleGroup) -> sizeof(*x)

jamienoss · 2018-08-08T17:19:35Z

pkg/acs/calacs/acsccd/doccd.c

    x = (SingleGroup *) malloc(acs_info->nimsets * sizeof(SingleGroup));
+    if (!x)
+       return (status = OUT_OF_MEMORY);
+    addPtr(&ptrReg, x, &free);


This is ok as is because the freeing happens in reverse order of that added. However, I'm wondering if this is "ok". Hmmm...

jamienoss · 2018-08-08T17:21:18Z

pkg/acs/calacs/acsccd/doccd.c

            return (status);
        }

        /* Read in keywords from primary header... */

-        if (GetKeyBool(x[i].globalhdr, "SUBARRAY", NO_DEFAULT, 0, &subarray))
+        if (GetKeyBool(x[i].globalhdr, "SUBARRAY", NO_DEFAULT, 0, &subarray)) {


if ((status = GetKeyBool(x[i].globalhdr, "SUBARRAY", NO_DEFAULT, 0, &subarray)))

jamienoss · 2018-08-08T17:23:28Z

pkg/acs/calacs/acsccd/doccd.c


    /* Get overscan region information from OSCNTAB */
+    int * overscan = NULL;
+    int * virtOverscan = NULL;
+    overscan = (int *) malloc(acs_info->nimsets * sizeof(int));


For maintenance reasons, it's better to remove the cast and also use sizeof(*overscan)

jamienoss · 2018-08-08T17:24:00Z

pkg/acs/calacs/acsccd/doccd.c

+    }
+    addPtr (&ptrReg, overscan, &free);
+
+    virtOverscan = (int *) malloc(acs_info->nimsets * sizeof(int));


jamienoss · 2018-08-08T17:30:55Z

pkg/acs/calacs/acsccd/doccd.c

+       This code is verbose and potentially harder to maintain, but the logic is clearer
+       for the scientist reading the code.
+
+       NOTE: The variable "done" is overloaded.  When done = overscan[i], this means


Can't we un-overload and instead of any if (done) use if (blevDone || overscan[i])? Not 100% certain of || vs && as I haven't read any of the below logic.

jamienoss · 2018-08-08T17:33:30Z

pkg/acs/calacs/acsccd/doccd.c

+    int driftcorr = NO;     /* true means bias level was corrected for drift */
+    Bool isSupportedSubAperture = NO;
+    float meanblev = 0.0;   /* mean value of overscan bias (for history) */
+    if (acs_info->blevcorr == PERFORM) {


if (!OmitStep(acs_info->blevcorr))

jamienoss · 2018-08-08T17:34:48Z

pkg/acs/calacs/acsccd/doccd.c

+    int driftcorr = NO;     /* true means bias level was corrected for drift */
+    Bool isSupportedSubAperture = NO;
+    float meanblev = 0.0;   /* mean value of overscan bias (for history) */
+    if (acs_info->blevcorr == PERFORM) {


I would pull this entire block out into a sep func. This would help clear up the crowded logic. Same prob goes for all the other steps.

jamienoss · 2018-08-08T17:42:27Z

pkg/acs/calacs/acsccd/doccd.c

+          }
+
+          /* Only Pre-SM4 WFC data - this is the original bias level subtraction */
+          if (acs_info->expstart < SM4MJD) {


A this stage of me reading this huge logic block(s) I would reduce all of them to calls. This would make things infinitely clearer.
E.g.

if (acs_info->expstart < SM4MJD) doSM4MJD(...); else doPostSM4MJD(...);

The nesting could also be described as a comment here at the root level. Maybe even inline e.g.

if (acs_info->expstart < SM4MJD) doSM4MJD(...); // -> if (subaray) // doSub(); else doPostSM4MJD(...);

Actually, perhaps it might be possible (and better?) to use a switch and order such that the logic exists as case stmts dropping through?

jamienoss · 2018-08-08T17:46:54Z

pkg/acs/calacs/acsccd/doccd.c

+                    */
+                    if (virtOverscan[0]) {
+                       isSupportedSubAperture = YES;
+                       trlmessage("This subarray data is supported for the bias shift correction.");


Do you want a break here?

jamienoss · 2018-08-08T17:55:17Z

pkg/acs/calacs/acsccd/doccd.c

+
+                 /* The variable done is set to the results of FindOver indicating there is
+                    physical overscan. */
+                 done = overscan[i];


Making this a note since you didn't edit doBlev(). It's better to not overload params, i.e. separate those that are input and those that are output. In this case it's done. This should either be past in (as a const) or a ptr so that it can be modified as output, but never both.

jamienoss

With the time given, I was not able to compare the logic to make a remark on whether it does the same as it does before (for the parts that we want this to be true for (whichever those are?)). Or even, whether it now functionally satisfies the issue it addresses.
That being said I don't think there are currently any bugs though it has introduced scope for future dev work to easily produce some, e.g. lack of NULL checking and variadic usage, etc.

Requesting changes, at least, on the grounds of the mallocs needing to be addressed. And, ptrs being accessed before being checked.

mdlpstsci · 2018-08-08T20:05:43Z

Addressing the mallocs and checking the sg[] pointers before actual use. New IT #334 opened which refer to the remaining issues in this PR.

…new bias level processing which previously only applied to full frame data.

jamienoss · 2018-08-09T19:41:45Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+  /* This function has built in assumptions regarding the data being passed */
+  assert (acs != NULL);  /* Make sure the ACSInfo structure is defined */
+  assert ((nGroups == 1) || (nGroups == 2)); /* Must be at least one and no more than two SingleGroups */
+  assert (((nGroups == 1) && (acs->subarray == YES)) || ((nGroups == 2) && (acs->subarray == NO)));  /* Subarray has one and full frame has two SingleGroups */


Just so I've said it - this and the line above are ugly. The variadic arg should have been removed instead and these asserts handled by the compiler (at compile time).

jamienoss · 2018-08-09T19:50:11Z

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c

+       for (i = 0; i < nGroups; i++) {
+           sg[i] = NULL;
+           sg[i] = va_arg (arguments, SingleGroup *);
+           if (!sg[i]) {


This is not what I had in mind. I meant that if va_arg (arguments, SingleGroup *) == NULL it just wouldn't be added, i.e.

SingleGroup * tmpGroup = va_arg (arguments, SingleGroup *); if (tmpGroup) sg[i] = tmpGroup; else continue;

You could argue that in such a case nGroups is actually incorrect, it is unclear of the use of the function params now that they are variadic. I.e. are they explicit slots, e.g. SingleGroup * amp2, SingleGroup * amp1 or is the order irrelevant and in the case that there is only one, it must always be the first, e.g. not passed as NULL, amp1.

I think the rest of the code assumes a specific ordering.

jamienoss

TBH I'd prefer not to approve this, however, the issues introduced will be addressed by #334 and there is significant pressure to merge this by today for the next release. I think the band-aids now applied should be temporarily sufficient. Oh well, here we go...

mdlpstsci added enhancement ACS New feature refactor labels May 29, 2018

mdlpstsci added this to the HSTDP-2018.3 milestone May 29, 2018

mdlpstsci self-assigned this May 29, 2018

mdlpstsci requested a review from pllim May 29, 2018 18:14

pllim reviewed May 29, 2018

View reviewed changes

mdlpstsci force-pushed the issue168-acs-subarrays branch 4 times, most recently from b401ee9 to 0fd4ffe Compare July 31, 2018 16:55

mdlpstsci force-pushed the issue168-acs-subarrays branch from 0fd4ffe to 63cbd7d Compare August 7, 2018 21:02

mdlpstsci changed the title ~~[WIP] Update ACS BLEVCORR routines to accommodate new supported subarrays~~ Update ACS BLEVCORR routines to accommodate new supported subarrays Aug 7, 2018

mdlpstsci requested a review from jamienoss August 8, 2018 12:23

mdlpstsci added the changes-data label Aug 8, 2018

mdlpstsci force-pushed the issue168-acs-subarrays branch from 63cbd7d to 6d5dc0d Compare August 8, 2018 14:05

jamienoss reviewed Aug 8, 2018

View reviewed changes

pkg/acs/calacs/acsccd/blev_funcs_postsm4.c Outdated

/* allocate space for true DC bias levels array */

PtrRegister ptrReg;

Copy link

Contributor

jamienoss Aug 8, 2018

Choose a reason for hiding this comment

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

👍

jamienoss reviewed Aug 8, 2018

View reviewed changes

jamienoss suggested changes Aug 8, 2018

View reviewed changes

mdlpstsci mentioned this pull request Aug 8, 2018

Review and address the issues discussed for bias shift correction for CALACS #334

Open

Updated routines to accommdate support subarray processing using the …

d2ef21f

…new bias level processing which previously only applied to full frame data.

mdlpstsci force-pushed the issue168-acs-subarrays branch from b91615a to d2ef21f Compare August 8, 2018 20:22

jamienoss reviewed Aug 9, 2018

View reviewed changes

jamienoss approved these changes Aug 9, 2018

View reviewed changes

mdlpstsci merged commit 0c2f4d6 into spacetelescope:master Aug 9, 2018

This was referenced Aug 9, 2018

Make acs->nimsets = findTotalNumberOfImsets() conditional on forwarModelOnly #335

Merged

Updated version in header files. #337

Merged

stscicrawford mentioned this pull request Feb 4, 2019

CALACS: Enable bias shift for new subarrays #168

Closed

jryon mentioned this pull request Apr 15, 2025

Extend the bias shift correction algorithm of CALACS to handle the WFC 512 and 1K subarrays #329

Open


		} /* End of only WFC detector */

		/* All HRC and SBC data - use the original blev correction */


		/* allocate space for true DC bias levels array */

		PtrRegister ptrReg;

Update ACS BLEVCORR routines to accommodate new supported subarrays #312

Update ACS BLEVCORR routines to accommodate new supported subarrays #312

Conversation

mdlpstsci commented May 29, 2018 • edited by pllim Loading

pllim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdlpstsci commented Aug 1, 2018

mdlpstsci commented Aug 7, 2018

mdlpstsci commented Aug 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

jamienoss left a comment

Choose a reason for hiding this comment

mdlpstsci commented Aug 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss left a comment

Choose a reason for hiding this comment

mdlpstsci commented May 29, 2018 •

edited by pllim

Loading

jamienoss Aug 8, 2018 •

edited

Loading

jamienoss Aug 8, 2018 •

edited

Loading

jamienoss Aug 8, 2018 •

edited

Loading

jamienoss Aug 8, 2018 •

edited

Loading

jamienoss Aug 8, 2018 •

edited

Loading

jamienoss Aug 8, 2018 •

edited

Loading