Review variable names associated with Kriging and GeoDataFrame names holding final results

[b-reyes]

In issue #34 we reviewed the names of key variables needed for the areal biomass density calculation and implemented them in PR #51. Now that we have established the names for the areal biomass density calculation, it is important that we also review the column names of the Kriging results GeoDataFrame.

Currently, the Kriging results are stored in the GeoDataFrame survey.bio_calc.krig_results_gdf, which have the following columns: centroid_latitude, centroid_longitude, Area (km^2), fraction_cell_in_polygon, geometry, krig_biomass_vp, krig_biomass_ep, krig_biomass_eps, area_calc, krig_biomass_vals.

• The columns names centroid_latitude, centroid_longitude, fraction_cell_in_polygon have already been decided in issue Implement variable name changes for the input data #19 and implemented in Change input data column names #43. Thus, we do not need to worry about these names.
• The column geometry should not be changed as it is an established name that is expected in a GeoDataFrame
• The column Area (km^2) is provided by the mesh data excel sheet. In issue Implement variable name changes for the input data #19 we did not discuss this name because it is data that is not used at all in the current EchoPro workflow. We can decide here if its inclusion is necessary and if so, what to name it.
• The columns krig_biomass_vp, krig_biomass_ep, krig_biomass_eps, area_calc, krig_biomass_vals are values created by the Kriging routine and we should decide what to name them.
  • In offline discussions it has been mentioned that having the prefix krig_ is not necessary as the results are in krig_results_gdf.

In addition to the column names, we can also discuss if the statement "Total Kriged Biomass Estimate" is appropriate for summing up the column krig_biomass_vals.

[emiliom]

Thanks for this great summary. One comment: in a previous discussion I believe we agreed that krig_biomass_vals should be renamed to biomass.

Also, since the suffixes in krig_biomass_vp, krig_biomass_ep, and krig_biomass_eps are pretty opaque, I'm pasting here what I found in https://github.com/uw-echospace/EchoPro/blob/master/EchoPro/kriging/kriging.py

• ep: Kriging variance
• eps: Kriging sample variance
• vp: Kriged value

[b-reyes]

• We will remove the column Area (km^2) from the mesh data input
• @emiliom will look into krig_biomass_vp, krig_biomass_ep, krig_biomass_eps, area_calc
• krig_biomass_vals -> biomass
• We will leave "Total Kriged Biomass Estimate" as it is

[emiliom]

A couple of comments about cell area:

• The parameter kriging_A0 is the fixed mesh cell area in nmi^2. It's set to 6.25 nmi^2 in initialization_config.yml, which is based on square cells that are 2.5 nmi x 2.5 nmi
• Area (km^2) really is in km^2. It's the actual cell area in km^2, where fraction_cell_in_polygon was applied externally to EchoPro. Of course, as we discussed, it's redundant and unnecessary.
• area_calc is identical to Area (km^2) but in nmi^2 and calculated in EchoPro (in kriging.run_biomass_kriging()) as $kriging\_A0 \times fraction\_cell\_in\_polygon$
• I suggest we rename area_calc to cell_area_nmi2, or actual_cell_area_nmi2 to be even more explicit.

[emiliom]

I've gone over a good chunk of the code and also referred back to Chu's documentation. It's not straightforward to match the kriging code to the equations in Chu's documents, and I won't claim I fully succeeded. But here are my recommendations about krig_biomass_vp, krig_biomass_ep, krig_biomass_eps and related names and decisions.

To restate what I had pasted above, the three core estimates produced by the kriging code, generically, are (stated as variable token : super-brief description):

• vp: Kriged value
• ep: Kriging variance
• eps: Kriging sample variance

Also relevant, in kriging._compute_kriging_vals and kriging.run_kriging, the quantity being kriged is generically referred to as field_data. I'll focus on these generic references first.

• "vp: Kriged value" is the estimate value of interest, a mean. I suggest that we use "mean" where "vp" occurs. For the generic reference, I suggest field_mean
• "ep: Kriging variance" is the modeled (theoretical?) variance estimated by the kriging procedure. A value is produced even when the corresponding field_mean is near zero. To clarify the source of this variance, I suggest field_krigvar (or field_krigvariance if we want to be very explicit, but see further comments below)
• "eps: Kriging sample variance" is the unbiased estimate of the population variance (the second central moment), calculated directly from the field data but also involving (weighing?) field_mean and field_krigvar (frankly, I'm not entirely sure what's going on). It's undefined if field_mean is 0 (division by zero), and thus set to Nan. I suggest field_samplevar.

A consequence of these variance calculations is that in the many mesh points where the kriged biomass density mean (field_mean) is zero, field_krigvar is defined and calculated but field_samplevar is Nan. You can see this in survey_2019.bio_calc.krig_results_gdf.

Ok, next and final step. The kriged quantity in kriging.run_biomass_kriging is areal_biomass_density_adult, strictly speaking. If we were to apply what I described above and replace "field" with "areal_biomass_density_adult", we'd come up with these awfully long but fully explicit variable and column names:

• krig_biomass_vp -> areal_biomass_density_adult_mean
• krig_biomass_ep -> areal_biomass_density_adult_krigvar
• krig_biomass_eps -> areal_biomass_density_adult_samplevar

We could go with those, or try to come up with an abbreviated form of "areal_biomass_density_adult". Long column names in dataframes make for unwieldy tabular displays, eg, when using df.head().

One more note: the units of "areal_biomass_density_adult" are kg / nmi^2. The units for biomass (the new name for krig_biomass_vals) are kg.

[emiliom]

While we're at it, I also recommend we rename all variables in kriging.py where the vp, ep and eps tokens occur. These include vp_val (a scalar) and vp_arr (an ndarray), and the same for ep and eps. More specifically, my tentative suggestions are: vp_val -> field_mean and vp_arr -> field_mean_arr

One more thing. While reviewing these and related issues, I looked through the code far and wide. I'd like to make two other renaming suggestions, to bring more consistency to user-facing variables central to the biomass calculations. The dataframes below are both in survey.bio_calc:

• final_biomass_table -> transect_biomass_gdf (it is a geodataframe)
• krig_results_gdf -> krig_biomass_gdf

Those changes to the dataframe names could happen in a separate PR, since they go beyond the scope of this PR.

[b-reyes]

I suggest we rename area_calc to cell_area_nmi2, or actual_cell_area_nmi2 to be even more explicit.

I like cell_area_nmi2 best. Should we be attaching units here?

Ok, next and final step. The kriged quantity in kriging.run_biomass_kriging is areal_biomass_density_adult, strictly speaking. If we were to apply what I described above and replace "field" with "areal_biomass_density_adult", we'd come up with these awfully long but fully explicit variable and column names:

• krig_biomass_vp -> areal_biomass_density_adult_mean
• krig_biomass_ep -> areal_biomass_density_adult_krigvar
• krig_biomass_eps -> areal_biomass_density_adult_samplevar

@emiliom thank you very much for taking a deep dive into this code. I think these names are a little too long also. From above, we will be changing krig_biomass_vals to biomass and we drop the adult suffix. So, we could do the same here. Additionally, not all of the names you suggest have krig in it. So, sticking with the theme above I think we should remove krig here too. How about the following name changes, which are slightly shorter?

• krig_biomass_vp -> areal_biomass_density_mean
• krig_biomass_ep -> areal_biomass_density_var
• krig_biomass_eps -> areal_biomass_density_samplevar

If we want to go even shorter, maybe we could remove areal?

While we're at it, I also recommend we rename all variables in kriging.py where the vp, ep and eps tokens occur. These include vp_val (a scalar) and vp_arr (an ndarray), and the same for ep and eps. More specifically, my tentative suggestions are: vp_val -> field_mean and vp_arr -> field_mean_arr

I am fine with renaming the variables in kriging.py too. I think there is a slight typo there, based on your comment it should be ep_val -> field_mean and ep_arr -> field_mean_arr.

One more thing. While reviewing these and related issues, I looked through the code far and wide. I'd like to make two other renaming suggestions, to bring more consistency to user-facing variables central to the biomass calculations. The dataframes below are both in survey.bio_calc:

• final_biomass_table -> transect_biomass_gdf (it is a geodataframe)
• krig_results_gdf -> krig_biomass_gdf
  Those changes to the dataframe names could happen in a separate PR, since they go beyond the scope of this PR.

I do see that renaming these could be useful. Here are my thoughts/suggestions:

• final_biomass_table: at the moment there is no biomass in this gdf. It only contains areal_biomass_density_adult, this name was previously created under the incorrect understanding that areal_biomass_density_adult was biomass. Thus, I think biomass should not be included in this name. In the future, I think we do want to add biomass to this gdf though. Perhaps it would be best to call this transect_gdf or transect_data_gdf.
• krig_results_gdf: this gdf contains other data than biomass, so I do not really like krig_biomass_gdf. Sticking with the theme of my previous bullet point, how about we do krig_data_gdf or kriging_data_gdf?

[emiliom]

I like cell_area_nmi2 best. Should we be attaching units here?

I believe units are generally not part of variable names in EchoPro. I added it here based on the "Area (km2)" column name (even if we're dropping it), and to be explicit w/o creating overly long variable / column names. Let's go with cell_area_nmi2

@emiliom thank you very much for taking a deep dive into this code. I think these names are a little too long also. From above, we will be changing krig_biomass_vals to biomass and we drop the adult suffix. So, we could do the same here. Additionally, not all of the names you suggest have krig in it. So, sticking with the theme above I think we should remove krig here too. How about the following name changes, which are slightly shorter?

• krig_biomass_vp -> areal_biomass_density_mean
• krig_biomass_ep -> areal_biomass_density_var
• krig_biomass_eps -> areal_biomass_density_samplevar

If we want to go even shorter, maybe we could remove areal?

I'm on board with shortening here the name areal_biomass_density_adult that we use elsewhere; and I agree that the parts we can drop are areal_, _adult or both. The main disadvantage of shortening the name here is that comparisons between final_biomass_table["areal_biomass_density_adult"] and, say, krig_results_gdf["areal_biomass_density_mean"] may seem a bit awkward. We can just live with that. Or revisit the previous decision to use areal_biomass_density, by removing the areal_ prefix everywhere. Looking at the code base, that would be a simple search and replace, "areal_biomass_density" -> "biomass_density". So, here's my recommendation:

• krig_biomass_vp -> biomass_density_adult_mean
• krig_biomass_ep -> biomass_density_adult_var
• krig_biomass_eps -> biomass_density_adult_samplevar
• In a separate PR, replace "areal_biomass_density" with "biomass_density" everywhere. For example, areal_biomass_density_adult will become biomass_density_adult.

I had recommended _krigvar rather than _var to be overly explicit about that variance, but let's go with your suggestion.

While we're at it, I also recommend we rename all variables in kriging.py where the vp, ep and eps tokens occur. These include vp_val (a scalar) and vp_arr (an ndarray), and the same for ep and eps. More specifically, my tentative suggestions are: vp_val -> field_mean and vp_arr -> field_mean_arr

I am fine with renaming the variables in kriging.py too. I think there is a slight typo there, based on your comment it should be ep_val -> field_mean and ep_arr -> field_mean_arr.

"vp" is the mean. I had a small mistake in one of my earlier comments, which I've corrected. So, the changes will be:

• vp: vp_val -> field_mean and vp_arr -> field_mean_arr
• ep: ep_val -> field_var and ep_arr -> field_var_arr
• eps: eps_val -> field_samplevar and eps_arr -> field_samplevar_arr

I do see that renaming these could be useful. Here are my thoughts/suggestions:

• final_biomass_table: at the moment there is no biomass in this gdf. It only contains areal_biomass_density_adult, this name was previously created under the incorrect understanding that areal_biomass_density_adult was biomass. Thus, I think biomass should not be included in this name. In the future, I think we do want to add biomass to this gdf though. Perhaps it would be best to call this transect_gdf or transect_data_gdf.
• krig_results_gdf: this gdf contains other data than biomass, so I do not really like krig_biomass_gdf. Sticking with the theme of my previous bullet point, how about we do krig_data_gdf or kriging_data_gdf?

I like the general thrust of your suggestion, to use a gdf base name that's broader than "biomass". My only change to your suggestion is to use "results" instead of "data", so it's clearer: transect_results_gdf and kriging_results_gdf. This change could be in a separate PR.

[b-reyes]

@emiliom thank you for your responses. I am in agreement with your suggestions.

To summarize, the following changes should be made (@emiliom please let me know if I missed anything):

• area_calc -> cell_area_nmi2
• We will remove the column Area (km^2) from the mesh data input
• krig_biomass_vals -> biomass
• We will leave "Total Kriged Biomass Estimate" as it is
• We will no longer include areal as a prefix. Thus, items such as areal_biomass_density_adult will become biomass_density_adult
• krig_biomass_vp -> biomass_density_adult_mean
• krig_biomass_ep -> biomass_density_adult_var
• krig_biomass_eps -> biomass_density_adult_samplevar
• vp: vp_val -> field_mean and vp_arr -> field_mean_arr
• ep: ep_val -> field_var and ep_arr -> field_var_arr
• eps: eps_val -> field_samplevar and eps_arr -> field_samplevar_arr
• final_biomass_table -> transect_results_gdf
• krig_results_gdf -> kriging_results_gdf

@emiliom these changes are pretty straightforward and I believe all of them can can be done in a single PR.

[b-reyes]

This issue has been addressed by PR #57, closing.