[EEE] Clean up FGF emissions calculation functions by matthew7838 · Pull Request #3237 · RuminantFarmSystems/RuFaS

matthew7838 · 2026-08-27T06:31:46Z

Refactors the repetitive farmgrown feed emissions code in EmissionsEstimator.

Context

Issue(s) closed by this pull request: closes #2651

What

Extracts the six per-variable accumulation blocks in _calculate_daily_farmgrown_feed_emissions_and_resources() into a single loop over a new _get_daily_emission_and_resource_values_for_field() helper.
Collapses the six copy-pasted reporting blocks in _report_daily_farmgrown_feed_fed_emissions_and_resources() into a loop over the new FARMGROWN_FEED_FED_OUTPUT_NAME_PREFIXES constant.
Replaces the repeated six-key dict literals with comprehensions over FARMGROWN_FEED_EMISSION_AND_RESOURCE_VARIABLES.

Why

Addresses the duplicate-code review notes from PR #2633.

How

The tracked variables and their output name prefixes are defined once at module level; calculation and reporting iterate over them. The refactor is behavior-preserving.

Test plan

Existing tests/test_EEE suite passes unchanged, including numeric fixture comparisons and add_variable_bulk call-count assertions.
Adds unit tests for the new helper.

Input Changes

N/A

Output Changes

N/A

github-actions · 2026-08-27T06:38:10Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1133
Mypy errors on dev branch: 1133
No difference in error counts

github-actions · 2026-08-27T06:39:00Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1133
Mypy errors on dev branch: 1133
No difference in error counts

github-actions · 2026-08-27T07:13:18Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1133
Mypy errors on dev branch: 1133
No difference in error counts

github-actions · 2026-08-27T08:02:12Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1133
Mypy errors on dev branch: 1133
No difference in error counts

ew3361zh · 2026-08-28T13:08:35Z

I think it would be worth coordinating this with the bug-fix #3202 that got merged into test but I believe still needs to get addressed on dev.

KFosterReed · 2026-08-28T16:10:43Z

I think it would be worth coordinating this with the bug-fix #3202 that got merged into test but I believe still needs to get addressed on dev.

Great catch Niko - I very much agree! I opened this issue about bringing both of the relevant updates to dev and also tried to start this PR #3230 for the emissions one in particular but coordinating this work with that one will make our lives a lot easier

github-actions · 2026-08-31T07:47:43Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1133
Mypy errors on dev branch: 1133
No difference in error counts

matthew7838 · 2026-09-01T07:43:29Z

@ew3361zh @KFosterReed I think, since this PR is low complexity, I will wait until after the test changes merge to resolve any conflicts, then re-request a review of this PR.

github-actions · 2026-09-07T07:58:57Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1164
Mypy errors on dev branch: 1164
No difference in error counts

ew3361zh

Looks good to me! Most fb centers around wanting more specificity in variable naming which I think would be helpful for future devs (and current devs when we need to look at the code again in 6 months).

ew3361zh · 2026-09-09T18:21:09Z

        farmgrown_feed_inventory_by_feed_id = self._gather_farmgrown_feed_inventory_data(all_simulation_days)

        for field_name in harvest_yield_by_field:
+            daily_values_by_variable = self._get_daily_emission_and_resource_values_for_field(


Suggested change

daily_values_by_variable = self._get_daily_emission_and_resource_values_for_field(

daily_emission_and_resource_values = self._get_daily_emission_and_resource_values_for_field(

ew3361zh · 2026-09-09T18:24:39Z

-                        "fertilizer_P": 0.0,
-                        "fertilizer_K": 0.0,
-                        "manure_N": 0.0,
+                        variable: 0.0 for variable in FARMGROWN_FEED_FED_OUTPUT_NAME_PREFIXES.keys()


Suggested change

variable: 0.0 for variable in FARMGROWN_FEED_FED_OUTPUT_NAME_PREFIXES.keys()

emission_or_resource: 0.0 for emission_or_resource in FARMGROWN_FEED_FED_OUTPUT_NAME_PREFIXES

ew3361zh · 2026-09-09T18:25:45Z

-                        start=0.0,
-                    )
-                    total_farmgrown_feed_emission_and_resource_by_feed_id[feed_id]["fertilizer_K"] += sum(
+                for variable, daily_values in daily_values_by_variable.items():


Suggested change

for variable, daily_values in daily_values_by_variable.items():

for emission_or_resource, daily_emission_and_resource_values in daily_emission_and_resource_values.items():

ew3361zh · 2026-09-09T18:30:10Z

+        Collects a field's daily emission and resource values for each tracked
+        emission and resource variable.


Suggested change

Collects a field's daily emission and resource values for each tracked

emission and resource variable.

Collects a field's daily emission and resource values.

ew3361zh · 2026-09-09T18:31:37Z

+        dict[str, dict[int, float]]
+            A nested dictionary structured as ``{variable: {simulation_day: value}}``
+            for each variable in ``FARMGROWN_FEED_FED_OUTPUT_NAME_PREFIXES``.
+            Variables whose application type has no data for the field map to empty


Suggested change

Variables whose application type has no data for the field map to empty

Variables whose application type (fertilizer or manure) has no data for the field map to empty

ew3361zh · 2026-09-09T18:34:50Z

-                    "fertilizer_P": data_for_feed_id_for_day["fertilizer_P"] * feed_deduction,
-                    "fertilizer_K": data_for_feed_id_for_day["fertilizer_K"] * feed_deduction,
-                    "manure_N": data_for_feed_id_for_day["manure_N"] * feed_deduction,
+                    variable: value * feed_deduction for variable, value in data_for_feed_id_for_day.items()


Same feedback as for the other - I would try to avoid the generic variable and value for this stuff since it is fairly specific and knowing what's what is helpful here.

github-actions · 2026-09-10T05:20:45Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1164
Mypy errors on dev branch: 1164
No difference in error counts

github-actions · 2026-09-10T06:13:05Z

Current Coverage: 99%

Mypy errors on clean-fgf branch: 1164
Mypy errors on dev branch: 1164
No difference in error counts

allisterakun

Clean behavior-preserving refactor! LGTM!

matthew7838 and others added 4 commits August 26, 2026 17:11

Refactor fgf

835b7c5

Updated changelog_WIP.md

c4509cc

Update changelog entry with PR number

ba45527

Merge ba45527 into fa7f57b

9356537

github-actions Bot force-pushed the clean-fgf branch from ba45527 to d7dc373 Compare August 27, 2026 06:33

Apply Black Formatting

f251c56

github-actions Bot force-pushed the clean-fgf branch from d7dc373 to f251c56 Compare August 27, 2026 06:34

matthew7838 changed the title ~~Clean up FGF emissions calculation functions~~ [EEE] Clean up FGF emissions calculation functions Aug 27, 2026

matthew7838 and others added 5 commits August 27, 2026 15:41

Remove inline comments

b1b05ba

Code quality cleanup and improvements

8649dee

Merge 8649dee into fa7f57b

dbfd9e1

Apply Black Formatting

776aea2

Merge 776aea2 into fa7f57b

61aa4e4

matthew7838 requested review from allisterakun and ew3361zh August 27, 2026 07:27

Apply Black Formatting

deac309

matthew7838 and others added 3 commits August 31, 2026 16:40

Merge branch 'dev' into clean-fgf

f0853b4

Merge f0853b4 into 25fffd5

84a5c2c

Apply Black Formatting

24ecb6f

matthew7838 and others added 3 commits September 7, 2026 16:52

Merge branch 'dev' into clean-fgf

7e0541d

Merge 7e0541d into 7b646dd

87dc238

Apply Black Formatting

a648957

ew3361zh mentioned this pull request Sep 8, 2026

[Patch][Branch Alignment] Update dev branch with test patch from PR 3214 #3256

Merged

ew3361zh approved these changes Sep 9, 2026

View reviewed changes

matthew7838 and others added 3 commits September 10, 2026 14:14

Merge branch 'dev' into clean-fgf

9edc9d2

Merge 9edc9d2 into fe94d62

2f7cb18

Apply Black Formatting

23da926

matthew7838 and others added 3 commits September 10, 2026 15:06

Addressed Niko's comments

2c1277a

Merge 2c1277a into fe94d62

86a28bd

Apply Black Formatting

2d3c71a

allisterakun approved these changes Sep 14, 2026

View reviewed changes

matthew7838 merged commit ca51145 into dev Sep 15, 2026
1 check passed

matthew7838 deleted the clean-fgf branch September 15, 2026 08:19

matthew7838 mentioned this pull request Sep 16, 2026

[EEE] Migrate farmgrown feed emissions fix from PR 3202 to dev #3288

Open

	daily_values_by_variable = self._get_daily_emission_and_resource_values_for_field(
	daily_emission_and_resource_values = self._get_daily_emission_and_resource_values_for_field(

	variable: 0.0 for variable in FARMGROWN_FEED_FED_OUTPUT_NAME_PREFIXES.keys()
	emission_or_resource: 0.0 for emission_or_resource in FARMGROWN_FEED_FED_OUTPUT_NAME_PREFIXES

	for variable, daily_values in daily_values_by_variable.items():
	for emission_or_resource, daily_emission_and_resource_values in daily_emission_and_resource_values.items():

		Collects a field's daily emission and resource values for each tracked
		emission and resource variable.

	Variables whose application type has no data for the field map to empty
	Variables whose application type (fertilizer or manure) has no data for the field map to empty

Uh oh!

Conversation

matthew7838 commented Aug 27, 2026

Context

What

Why

How

Test plan

Input Changes

Output Changes

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

ew3361zh commented Aug 28, 2026

Uh oh!

KFosterReed commented Aug 28, 2026

Uh oh!

github-actions Bot commented Aug 31, 2026

Uh oh!

matthew7838 commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 7, 2026

Uh oh!

ew3361zh left a comment

Choose a reason for hiding this comment

Uh oh!

ew3361zh Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

ew3361zh Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

ew3361zh Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

ew3361zh Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

ew3361zh Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

ew3361zh Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

allisterakun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants