[Animal][Ration] Refactor Pen.formulate_optimized_ration() by matthew7838 · Pull Request #3043 · RuminantFarmSystems/RuFaS

matthew7838 · 2026-05-28T13:42:51Z

Refactor Pen.formulate_optimized_ration() to remove the # noqa: C901 complexity suppression. Reduces complexity from 13 to 2 by extracting four focused helpers, hoists magic values into named class constants, and tightens a few adjacent rough edges in _attempt_formulation.

Context

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

What

Removed # noqa: C901 from formulate_optimized_ration.
Extracted four new private helpers:
- _run_formulation_attempts — the entire while-loop driving repeated optimization attempts.
- _maybe_increased_dmi_for_retry — returns a bumped DMI value (or None) when a user-defined-ration retry is warranted.
- _handle_lactation_failure_in_loop — dispatches between the user-defined vs. auto lactation-failure handlers and returns whether the loop should stop.
- _finalize_ration_outcome — handles the post-loop apply / log / error chain.
Added two class-level constants:
- _DMI_INCREASE_CONSTRAINTS — frozenset of the constraint names that signal a DMI retry could help (was built inline every loop iteration).
- _DMI_RETRY_INCREASE_FACTOR = 1.1 — replaces the bare magic number.
Renamed the confusing pair initial_dry_matter_requirement / initial_dry_matter_requirement_fixed → current_dmi_requirement / original_dmi_requirement. The new names tell you which one is the loop-mutable copy and which is the snapshot.
Dropped the defensive getattr(self, "ration", None) in favor of direct self.ration — the attribute is always set by init, so the fallback was dead code that obscured intent and broadened the type.
Cleaned up _attempt_formulation:
- Replaced two 4-line if/else blocks (user_defined_ration_dictionary, tolerance) with one-liner ternaries.
- Replaced list(self.animals_in_pen.values())[0] with next(iter(...)) (no throwaway list).
- Extracted the NASEM averaging block into a new _pen_nasem_averages() helper.
- Added the missing temperature parameter to the docstring.

Why

formulate_optimized_ration had a McCabe complexity of 13 (suppressed with # noqa: C901), making it hard to read, hard to test in isolation, and hard to extend.

How

Reduced complexity.

Test plan

Run with updated unit tests.
e2e testing assuring no output change.

Input Changes

Output Changes

N/A

Filter

github-actions · 2026-05-28T13:49:24Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1155
Mypy errors on dev branch: 1155
No difference in error counts

github-actions · 2026-05-28T13:49:25Z

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.

github-actions · 2026-05-28T13:49:48Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1155
Mypy errors on dev branch: 1155
No difference in error counts

github-actions · 2026-05-28T13:49:49Z

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.

github-actions · 2026-06-03T08:26:12Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1145
Mypy errors on dev branch: 1145
No difference in error counts

github-actions · 2026-06-03T08:26:13Z

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.

Resolved two docstring conflicts in RUFAS/biophysical/animal/pen.py: - remove_animals_by_ids: kept origin's Notes block (O(n) cost explanation) - update_animal_combination: kept HEAD's full-sentence description Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-06-03T08:34:23Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1145
Mypy errors on dev branch: 1145
No difference in error counts

github-actions · 2026-06-03T08:34:24Z

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.

github-actions · 2026-06-03T15:31:19Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1145
Mypy errors on dev branch: 1145
No difference in error counts

tomhuhh

Human review passed too, with one caveat.

The outputs remained identical for 3 of the 4 example diets tested. See the summary plot below.

NE diet failed because 176 is missing from the top-level available feeds list. Please fix this before merging.

github-actions · 2026-06-16T14:40:39Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1144
Mypy errors on dev branch: 1144
No difference in error counts

github-actions · 2026-06-16T14:40:40Z

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.

matthew7838 · 2026-06-16T14:51:13Z

Human review passed too, with one caveat.

The outputs remained identical for 3 of the 4 example diets tested. See the summary plot below.
NE diet failed because 176 is missing from the top-level available feeds list. Please fix this before merging.

@tomhuhh , can you elaborate a little bit more on this? I'm a little bit confused about exactly where to add the top-level available feeds(which input file?). Also, I'm assuming it's missing/failing for both dev and this branch from looking at the graph, is that right? Or is it something I removed on this PR?

JoeWaddell

This looks great! Tested on a few rations and it performs just as it does on dev. Good job tidying this up.

JoeWaddell · 2026-06-16T14:50:11Z

+    # Constraint failures that indicate a lactating-cow ration retry should bump the dry matter intake.
+    _DMI_INCREASE_CONSTRAINTS: frozenset[str] = frozenset(
+        {
+            "NE_total_constraint",
+            "NE_maintenance_and_activity_constraint",
+            "NE_lactation_constraint",
+            "NE_growth_constraint",
+            "calcium_constraint",
+            "phosphorus_constraint",
+            "protein_constraint_lower",
+            "DMI_constraint_lower",
+        }
+    )
+    # Multiplier applied to the dry matter intake when a lactating-cow ration retry is warranted.
+    _DMI_RETRY_INCREASE_FACTOR: float = 1.1


I also wonder if the _DMI_RETRY_INCREASE_FACTOR should be defined in animal_module_constants.py

JoeWaddell · 2026-06-16T16:54:05Z

Human review passed too, with one caveat.
The outputs remained identical for 3 of the 4 example diets tested. See the summary plot below.

NE diet failed because 176 is missing from the top-level available feeds list. Please fix this before merging.

@tomhuhh , can you elaborate a little bit more on this? I'm a little bit confused about exactly where to add the top-level available feeds(which input file?). Also, I'm assuming it's missing/failing for both dev and this branch from looking at the graph, is that right? Or is it something I removed on this PR?

It's also failing on dev! Needs the pricing/buffer info. Tested the example Northeast feed file and found it as well.

github-actions · 2026-06-17T12:14:44Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1144
Mypy errors on dev branch: 1144
No difference in error counts

github-actions · 2026-06-17T12:14:45Z

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.

github-actions · 2026-06-17T12:32:22Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1144
Mypy errors on dev branch: 1144
No difference in error counts

github-actions · 2026-06-17T12:32:23Z

🚨 Please update the changelog. This PR cannot be merged until changelog.md is updated.
🚨 Unauthorized changes detected in protected files. Please remove these changes if they are not intended.

github-actions · 2026-06-17T12:40:03Z

Current Coverage: 99%

Mypy errors on refactor-pen branch: 1144
Mypy errors on dev branch: 1144
No difference in error counts

github-actions · 2026-06-17T12:40:04Z

🚨 Unauthorized changes detected in protected files. Please remove these changes if they are not intended.

matthew7838 and others added 5 commits May 25, 2026 13:07

Initial changes

c94159b

Updated tests

0f83738

Updated tests

b3fd01a

Merge branch 'dev' into refactor-pen

38ff11c

Merge 38ff11c into 60aa035

e2d893f

github-actions Bot force-pushed the refactor-pen branch from 38ff11c to ffb44f8 Compare May 28, 2026 13:44

Apply Black Formatting

d3cb123

github-actions Bot force-pushed the refactor-pen branch from ffb44f8 to d3cb123 Compare May 28, 2026 13:45

matthew7838 and others added 4 commits June 3, 2026 17:19

Styling changes

f4235b9

Merge branch 'dev' into refactor-pen

29839b5

Merge 29839b5 into ae23cfb

4373082

Apply Black Formatting

ae4184d

matthew7838 and others added 4 commits June 3, 2026 17:27

Merge remote-tracking branch 'origin/refactor-pen' into refactor-pen

47cceab

Merge 47cceab into ae23cfb

f24de4c

Apply Black Formatting

9839d53

matthew7838 marked this pull request as ready for review June 3, 2026 09:18

matthew7838 requested a review from JoeWaddell June 3, 2026 09:19

matthew7838 and others added 3 commits June 4, 2026 00:24

Merge branch 'dev' into refactor-pen

f22ce8c

Merge f22ce8c into 2c9eb8c

378d373

Apply Black Formatting

4ebdd1d

tomhuhh approved these changes Jun 15, 2026

View reviewed changes

matthew7838 and others added 3 commits June 16, 2026 23:33

Merge branch 'dev' into refactor-pen

a69123e

Merge a69123e into bd22780

a85ed20

Apply Black Formatting

7b64252

JoeWaddell approved these changes Jun 16, 2026

View reviewed changes

matthew7838 and others added 3 commits June 17, 2026 21:08

Merge branch 'dev' into refactor-pen

5c13b6a

Merge 5c13b6a into 0d1e886

b4b2d0f

Apply Black Formatting

b3c5080

matthew7838 and others added 8 commits June 17, 2026 21:23

Addressed Allister's comments

545db82

Patched missing available feed 176 in NE feed

9e7f29f

Merge remote-tracking branch 'origin/refactor-pen' into refactor-pen

8d49981

Merge 8d49981 into 0d1e886

460136f

Apply Black Formatting

88b0e93

Updated changelog.md

8cdd43d

Merge remote-tracking branch 'origin/refactor-pen' into refactor-pen

58c173e

Merge 58c173e into 0d1e886

f4588bd

matthew7838 self-assigned this Jun 17, 2026

Apply Black Formatting

cb27846

matthew7838 merged commit 67665e6 into dev Jun 17, 2026
1 check passed

matthew7838 deleted the refactor-pen branch June 17, 2026 12:41

Conversation

matthew7838 commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

What

Why

How

Test plan

Input Changes

Output Changes

Filter

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

tomhuhh left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

matthew7838 commented Jun 16, 2026

Uh oh!

JoeWaddell left a comment

Choose a reason for hiding this comment

Uh oh!

JoeWaddell Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

JoeWaddell commented Jun 16, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matthew7838 commented May 28, 2026 •

edited

Loading