Harmonize new recursive functions with those in recursive-refac

[catherinebirney]

Discuss steps necessary to update the FBS methodology to use recursive modeling
@matthewlchambers @bl-young

• Flag new functions vs existing functions
• Improve/update documentation of FBS methods

[bl-young]

• Drop new FBS methods that are not designed to be core methods (and activitysets)

[bl-young]

Status of FBS:

• GRDREL_national (no diff)
• GRDREL_state (no diff)
• TRI_DMR_national (diff) - reflects changes in how MetaSources are aggregated (they are not in new branch, so more records in the final FBS); no difference in quantities
• TRI_DMR_state
• CRHW_national (no diff)
• CRHW_state
• CAP_HAP_national (requires significant refactoring, currently causing memory error)
• CNHW_CA (no diff)
• CNHW_national (no diff)
• CNHW_state (no diff)
• CNHWC_national (no diff)

[matthewlchambers]

What do the checkmarks refer to here? Just wondering since the GRDREL_national, at least, doesn't run using the new code.

[bl-young]

@matthewlchambers Yes I did link to the actions confirming being able to run them. I'm surprised you're not able to run GRDREL_national...

On recursive-refac I can succesfully run

    m = 'GRDREL_national_2017'
    flowsa.flowby.FlowBySector.generateFlowBySector(m)

[bl-young]

But yes the checkmarks reflect successful completion of the method update (also trying to note whether the results differ or not, and perhaps why)

[matthewlchambers]

Ah, I see. That runs the old code, not the new code.

EDIT: We determined that this assertion is false.

[bl-young]

Ah, I see. That runs the old code, not the new code.

I'm not sure I follow. I'm able to run GRDREL_national with your new updates with generateFlowBySector(). What do you mean by the new code?

[bl-young]

Right- the code above uses flowby

[matthewlchambers]

Well I feel silly, haha. I misread it somehow. My apologies!

[bl-young]

Haha no problem! But does that still mean you are unable to run it?

[matthewlchambers]

Solved the issue. I just needed to delete some of the saved stewi parquets (my local copy was from last year or something like that). Once I did that everything worked fine!

[matthewlchambers]

Just pushed a fix for this issue.

[matthewlchambers]

Just pushed a fix for this issue.

[bl-young]

thank you! I will try out both

[matthewlchambers]

Regarding CAP_HAP_national_2017, is there a need for using QCEWs from different years? It would be much simpler to just use a single year.

[bl-young]

Its an old helper source, so its used to expand EIA_MECS (2014 or 2018) to 6-digit NAICS, and also used to expand BEA data (2012) down to 6-digit NAICS. Are you attending today's flowsa meeting? We could put it on the agenda to discuss then.

Also there has been discussion of moving to an employment FBS instead of BLS_QCEW (aka an already processed version) that would speed things up substantially and make caching less necessary. @catherinebirney

[catherinebirney]

I think we can go ahead and use the existing Employment FBS because those employment FBS are created using the same steps we use to clean up BLS data every time it is called. We discussed filling in some of the suppressed QCEW data with QWI data, which we haven't done yet.

The issue is that with the original method, it really wasn't set up to use FBS as an allocation source for FBAs. So depending on how Matthew's branch is set up, we could go ahead and make that switch. Although we need to update the employment FBS to use Matthew's revised method to estimate suppressed data.

[matthewlchambers]

There shouldn't be any issue with using an FBS as an allocation source in the recursive-refac branch, and it would be an especially good idea for anything below the national geoscale, since it takes a looong time to clean the QCEW at a state or county level.

[bl-young]

since it takes a looong time to clean the QCEW at a state or county level.

I'm two hours in... https://github.com/USEPA/flowsa/actions/runs/3378018922/jobs/5607673435

[catherinebirney]

@matthewlchambers @bl-young
In the recursive version, when an FBA is mapped to the fed elem flow list here, the FBA columns "FlowName" and "Compartment" are dropped. The next step is to use the "clean_fba" function.

In the original method, those columns are retained .

In the water FBS method I'm running into an issue with the recursive method that the "clean_fba" functions subset data using FlowName/Compartment. I can update the functions to use Flowable/Context info, but before I do I wanted to check if this is a widespread issue and if it is worth retaining FlowName/Compartment and dropping those columns later.

[bl-young]

So this specifically is for methods where the flows ARE mapped to FEDEFL, but we also want to maintain the original names for some reason? I am trying to remember if that is used in other methods or not...

[catherinebirney]

Correct - and I believe the original columns are maintained just for use in the "clean_fba" function and then not called on again. I'll do a little digging, but will likely update the water clean_fba to use Flowable/Context. This issue is the reason GH actions are failing.

[bl-young]

noting that approach for mapping and standardizing was revised 651b62f

[catherinebirney]

FBS status:

• Employment_national_2017
• Employment_national_2018
• Employment_state_2012
• Employment_state_2013
• Employment_state_2014
• Employment_state_2015
• Employment_state_2016
• Employment_state_2017
• Employment_state_2018
• Employment_state_2019
• Employment_state_2020
• Land_national_2012
• Water_national_2010_m1
• Water_national_2010_m2
• Water_national_2015_m1
• Water_national_2015_m2
• Water_national_2015_m3
• Water_state_2015_m1

[catherinebirney]

The national employment FBS run locally, but run into a memory failure on GH actions

[bl-young]

Potential feature:
Could we cache a partially processed dataset but still allow for additional selection_fields after its pulled from the cache?

The specific application I'm thinking is when we use EIA_MECS but need to subset for different flows. Estimating the suppressed data would only then be required once, instead of each time that attribution source is called.

flowsa/flowsa/flowby.py

Lines 1005 to 1008 in 651b62f

	# First check cache for FlowBy
	if isinstance(fba.config['attribution_source'], str):
	attribution_fbs = (fba.config['cache']
	[fba.config['attribution_source']])

I think that here we could just append select_by_fields() once we ensure the configuration is set up appropriately (would need to change how we check if a dataset is cached since the isinstance(fba.config['attribution_source'], str) would not longer apply)

[bl-young]

see #294

[matthewlchambers]

#294 has now been merged, so we now treat the cached source as any other FlowBySector (and note that even if the cached source was originally an FBA, it will have been converted to an FBS before being cached), so we can select by fields or apply cleaning functions or anything else we could do with an FBS attribution source.

[bl-young]

Yes, I noticed that I had to change the select_fields for flows to be Flowable instead of FlowName for instance in my testing. I plan to push a revised CAP_HAP soon

[bl-young]

FlowBySectors seem to generate with extra FBA columns such as:

fbs.columns
Out[12]: 
Index(['Flowable', 'Class', 'SectorProducedBy', 'SectorConsumedBy',
       'SectorSourceName', 'Context', 'Location', 'LocationSystem',
       'FlowAmount', 'Unit', 'FlowType', 'Year', 'MeasureofSpread', 'Spread',
       'DistributionType', 'Min', 'Max', 'DataReliability',
       'TemporalCorrelation', 'GeographicalCorrelation',
       'TechnologicalCorrelation', 'DataCollection', 'MetaSources', 'FlowUUID',
       'ActivityProducedBy', 'ActivityConsumedBy', 'SourceName', 'Description',
       'ProducedBySectorType', 'ConsumedBySectorType'],
      dtype='object')

specifically: ActivityProducedBy, ActivityConsumedBy, SourceName, Description, but all fields are blank
is that by design @matthewlchambers

[bl-young]

this seems specific to CNHW_CA_2014

[matthewlchambers]

It's definitely not intentional. And it appears to be a more general issue (at the very least, it's also happening with GHGI_recursive_2017_m1).

[matthewlchambers]

I'll have to figure out what's happening here. Thanks for pointing it out!

[matthewlchambers]

Ok, see #295 for a fix.

[matthewlchambers]

Merged #295 into recursive-refac.

[catherinebirney]

@matthewlchambers @bl-young The current hold up with converting Water_national_m1 to the recursive method is that the 'clean_fba' function is called on for each activity set individually, rather than applying 'clean_fba' to the entire source df before subsetting the df into activity sets (which is the order in the original method). For USGS_NWIS_WU, the df needs to be cleaned before broken into activity sets because there are some calculations that require activities that are in different activity sets.

@matthewlchambers Is there a reason 'clean_fba' is called on repeatedly rather than a single time? Can we call on clean_fba before each activity set?

[bl-young]

would this work: clean_fba_before_activity_sets

[catherinebirney]

@bl-young - yes, thank you!

[matthewlchambers]

Just pushed an update to recursive-refac changing how primary and secondary columns are assigned when we specify primary_action_type. Specifically, I realized that the primary column was being assigned in an efficient way, but the secondary column was being assigned inefficiently. Now, both columns are assigned efficiently if primary_action_type is specified, and it takes 10-15 seconds to create each activity set in CAP_HAP_national_2017. Let me know if you encounter any issues.

[bl-young]

oh nice - thanks! i'll check it out

[bl-young]

this is making my life so much easier 🙌

[bl-young]

The crop data in NEI via USDA uses disaggregate_coa_cropland_to_6_digit_naics. @matthewlchambers is that one that you've converted yet? I also see for GHGI you are using a second attribution of USDA_CoA_Cropland_NAICS, is that the best model to follow?

[matthewlchambers]

What disaggregate_coa_cropland_to_6_digit_naics seems to be doing is using USDA_CoA_Cropland_NAICS as an attribution source. So yes, I think just explicitly using USDA_CoA_Cropland_NAICS as a secondary attribution source is better if possible. If there's something more going on with disaggregate_coa_cropland_to_6_digit_naics than I realized, it can probably still be implemented in the context of USDA_CoA_Cropland_NAICS as a secondary attribution source.

[matthewlchambers]

For example, as a clean_fba function for USDA_CoA_Cropland_NAICS.

[bl-young]

@matthewlchambers I'm experiencing an issue with CAP_HAP when trying to use BEA Use table data to allocate an activity set (e.g. EPA_NEI_Nonpoint.mobile_and_fuel_storage). Perhaps this was not relevant for the GHG data because both the primary source and attribution source are the same geoscale.

flowsa/flowsa/flowby.py

Lines 901 to 917 in eb3724a

	highest_reporting_level_by_geoscale = [
	(self
	.merge(geoscale_by_fips, how='inner')
	.query('geoscale <= @scale')
	.groupby(['ActivityProducedBy', 'ActivityConsumedBy']
	+ [s.name.title() for s in geo.scale
	if s.has_fips_level and s >= scale],
	dropna=False)
	.agg({'geoscale': 'max'})
	.reset_index()
	.rename(columns={
	'geoscale': f'highest_reporting_level_by_{scale.name.title()}'
	}))
	.astype(object)
	for scale in geo.scale
	if scale.has_fips_level and scale <= target_geoscale
	]

When creating the highest_reporting_level_by_geoscale from BEA, one of the Activity columns is typically "" due to the collapse of the columns:

flowsa/flowsa/methods/flowbysectormethods/CAP_HAP_national_2017.yaml

Lines 160 to 161 in eb3724a

	selection_fields:
	ActivityProducedBy: {'324110': ''} # Petroleum

In line 915 in the chunk above, converting to .astype(object) seems to convert those "" into nan. Then upon subsqueently merging with the FBA, the merge fails due to those nan.

Notably adding a .fillna('') solves this problem, but I expect that is not a good fix. Is .astype(object) necessary here?

[bl-young]

need to confirm if this is still an issue.

[catherinebirney]

I'm back to having an issue here for the national water FBS because some of the data has info in both the FBA and FBS columns while other data only has info in one column

[catherinebirney]

The solution I came up with is to convert the activity column dtypes only if the dtype is float. Even though the activity columns are set to object when the df is subset by geoscale, one of the water FBS activity col dtypes is reset to float because the column is subset from a mix of string/np.nan to only np.nan. So I first check if the col dype is float and if it is convert it. I don't get an error when running CAP_HAP, but CAP_HAP data is coming into the geoscale function with np.nan instead of '', so I think there is an upstream issue.

[catherinebirney]

@bl-young @matthewlchambers see if CAP HAP is still a problem

[bl-young]

I now get a memory error when I try to run the whole thing. But I am able to get through successfully these activity sets without error

[bl-young]

I'm having an issue in convert_to_geoscale() for stewi data where the dataset (e.g. RCRAInfo in CRHW_national_2017) comes in at both state and county level. Unlike in some other datasets (e.g. BLS data) where both state and county are reported separately (and generally these are considered duplicative), in some stewi datasets we do not have full location information for a facility and can only assign a state but not county. Therefore a full complete non-duplicative dataset will have both state and county records.

Specifically data are getting dropped here because the geoscale == source_geoscale line

flowsa/flowsa/flowby.py

Lines 934 to 944 in 0d0e361

	fba_at_source_geoscale = (
	fba_with_reporting_levels
	.assign(source_geoscale=(
	fba_with_reporting_levels[reporting_level_columns]
	.max(axis='columns')))
	# ^^^ max() with axis='columns' takes max along rows
	.query('geoscale == source_geoscale')
	.drop(columns=(['geoscale',
	*geoscale_name_columns,
	*reporting_level_columns]))
	)

any ideas @matthewlchambers

[bl-young]

I should note that this stems from an effort to update stewi based datasets to use flowby functions and the new classes

[bl-young]

since this likely applies only to point source datasets (e.g. stewi), handle this in stewi by converting to the appropriate geoscale all entries within that function)

[bl-young]

This has been fixed here: 3fcc451; handled within stewi_fbs.py

[bl-young]

Duplicate FIPS appearing in FIPS_geoscale 51019

I expect this stems from a FIPS getting split across the time series but have not yet confirmed. I also have not yet confirmed if this is an issue in the main branch.

[matthewlchambers]

What years are involved here? Bedford (city) switched from being an independent city (FIPS code 51515; not included in any county) to being part of Bedford county (FIPS code 51019) in 2013. Virginia's practice of making incorporated cities explicitly not part of any county makes any sort of time series county-level data interesting, to say the least.

[bl-young]

Ahh yes as a Virginia resident this is not the first time i've heard about Virginia causing real problems for data science! :)
I believe this was the default (so 2015?) and I think it's due to a merge across years at some point.

[catherinebirney]

We do have a FIPS crosswalk which could be used to align the years

[bl-young]

Yes I think that's the issue Bedford shows up twice in the crosswalk (line 2879 and 2966) because of the two fips merging

[matthewlchambers]

That could indeed do it. We may need to revisit how the crosswalk is applied and make sure we're getting our FIPS codes from the correct year.

[catherinebirney]

@matthewlchambers @bl-young
The original FBS methods were set up to have a target geoscale and then within a data source, we could specify which geoscale to use. This option is necessary for cases like the water method where the objective is a national level dataset, but we want to use state level data to get there rather than published national data. Now if within a data source we specify a geoscale (see water m1), that specified geoscale overrides the target geoscale listed at the top of the yaml. I plan to modify the code so the top level geoscale is still the target - I don't think this should cause issues with other FBS methods and Ben, I believe you need this option for the CAP_HAP method

[matthewlchambers]

Ok, then my first thought is that we can specify state geoscale for both the activity set and the attribution source (illustration below). This will result in an FBS with different geoscales for different activity sets, but we already needed to aggregate the different activity sets up anyway.

# Top of document. Keys irrelevant to the current discussion omitted
geoscale: national
source_names:
  Arbitrary_Source:
    activity_sets:
      arbitrary_activity_set:
        geoscale: state
        attribution_method: proportional
        attribution_source:
          arbitrary_attribution_source:
            geoscale: state

[catherinebirney]

Do you mean the final FBS output for a "national" level FBS will have a mix of national and state data (for example) based on if we specify a geoscale in an activity set? If I'm understanding your comment correctly, we don't want to do that. For some of the FBS methods we want to create both national and state level versions. If the "national" version has some state-level data, there is a chance it won't align perfectly with the "state" FBS version and that would be confusing. And the objective is to have a national level FBS. Can we add an additional line of code at the end of the FBS build to get the top level geoscale and aggregate up any activity sets that do not meet it? I

[matthewlchambers]

I should also point out that the recursive nature of the configuration file means that specifying geoscale: state is actually unnecessary for the attribution source if it is specified on the activity set. So the example above should more properly read:

# Top of document. Keys irrelevant to the current discussion omitted
geoscale: national
source_names:
 Arbitrary_Source:
   activity_sets:
     arbitrary_activity_set:
       geoscale: state
       attribution_method: proportional
       attribution_source:
         arbitrary_attribution_source:

[matthewlchambers]

Do you mean the final FBS output for a "national" level FBS will have a mix of national and state data (for example) based on if we specify a geoscale in an activity set? If I'm understanding your comment correctly, we don't want to do that. For some of the FBS methods we want to create both national and state level versions. If the "national" version has some state-level data, there is a chance it won't align perfectly with the "state" FBS version and that would be confusing. And the objective is to have a national level FBS. Can we add an additional line of code at the end of the FBS build to get the top level geoscale and aggregate up any activity sets that do not meet it? I

I think that should be very doable. In fact, it might be the case that the existing FB convert_fips_to_geoscale() method, combined with aggregate_flowby() would obtain something close to the desired result (if provided with the correct arguments, including the set of column to group by in the case of aggregate-flowby()). However, I think the preferred approach would be to implement an FBS version of the convert_to_geoscale method, currently only implemented for FBA.

[catherinebirney]

added method of aggregating activity geoscale to target geoscale with 70d4125

[catherinebirney]

@matthewlchambers I have another Water FBS specific issue. The target sector is 6-digits, however some of the data are brought in at 7-digit sectors and I want to keep them at NAICS7 until after 2 datasets are multiplied. I am multiplying a water application rate (kg/m2) by total land area (m2) for specific crops. Aggregating to NAICS6 before multiplying would result in less accurate calculations of water use by crop type. Additionally because one of the data sets is a rate (kg/m2), that dataset cannot be aggregated from NAICS7 to NAICS6 by simply summing the data, it would need to be a weighted aggregation.

[matthewlchambers]

I think this can be approached in the same way as above, by specifying a different industry_spec for the activity set (which will also be applied to the attribution source), then aggregating afterward.

[catherinebirney]

I was thinking the same. There is still an issue of aggregating "rates' which right now I think is only in the water FBS. It might be worth adding a unit check and if the unit appears to be a rate with "/" we add a warning.

[catherinebirney]

added method of aggregating activity sets to target sector level with 0271c76

[catherinebirney]

@matthewlchambers A dataset with text based activities that could benefit from estimating suppressed data is in Water_national_2015_m1, "cropland_attribution", the "USDA_CoA_Cropland" FBA used for attribution.

Right now, all suppressed data in USDA_CoA_Cropland is dropped in convert_to_geoscale() because that function is called before attribute_flows_to_sectors() and we aggregate the FBA in convert_to_geoscale(). What do you think of reorganizing those two function calls in prepare_fbs()?

[matthewlchambers]

I see what you're saying. Yes, I agree that suppressed data should be estimated (which as you've noted requires mapping to sectors) before we convert the geoscale. Not least because some methods of estimating suppressed data might use data from another geoscale to do so.

[matthewlchambers]

Starting to work on this issue. While we're renaming aggregated to parent-child and disaggregated to flat, could we also consider renaming sector-like_activites to naics-like_activities? This would be both clearer and more accurate to the present state of things.

Actually, what might be even better would be to use the existing activity_schema key. Anytime we would have checked whether sector-like_activities is True, we could instead check whether activity_schema starts with 'NAICS'. I kind of favor this approach because even if we later add other activities schemas which are sector-like, we probably can't use the existing code for "sector-like activities", since the existing code in these cases is entirely NAICS focused.

[catherinebirney]

I like the suggestion to use activity_schema and move away from sector-like_activities. Those are duplicative parameters in a sense. Could we use a dictionary to assign schema by data year? We already have some NAICS 2017 schemas, but only for certain years. It would help if we could say EIA data from 2010-2016 uses NAICS_2012 while data from 2017+ uses NAICS_2017. Just an example - I'd have to look at the data again.

As an aside, we updated aggregated to flat and disaggregated toparent-child - goes to show how confusing those labels were :)

[matthewlchambers]

I like the idea of using a dictionary to assign schema by data-year, though there may be some implementation challenges.

[matthewlchambers]

Another question related to fixing this overall issue: could we switch to requiring crosswalks to actually specify their SectorSourceName? (Generally this amounts to specifying the relevant NAICS year.) This would clean up some code that currently tries to handle situations where that is missing. I think just giving an appropriate error so the user can fix their crosswalk would be better.

[catherinebirney]

@matthewlchambers in the water FBS, I want to attribute a single activity "Irrigation Crop" in the primary data source to NAICS 6. To do so, I call on USDA_CoA_Cropland. The USDA_CoA_Cropland needs to be first further attributed using USDA_CoA_Cropland_NAICS. But what I would like to do is attribute all USDA_CoA_Cropland sectors that begin with '112' with UsDA_CoA_Cropland_NAICS that have a "FARM OPERATIONS" flowclass and all USDA_CoA_Cropland sectors that begin with '111' with USDA_CoA_Cropland_NAICS that have a "AG LAND, CROPLAND, HARVESTED" flowclass. The FBS methodology is not currently set up to attribute part of an attribution source via one method and another part via another method, correct? I can't break the primary data source into two activity sets, so really I need a way of breaking the attribution source into 2 activity sets.

[matthewlchambers]

You can definitely split an attribution source into activity sets :-) It's one of the nice features of the recursive system.