Skip to content

Semen Allocation#2966

Draft
allisterakun wants to merge 23 commits into
devfrom
semen_allocation
Draft

Semen Allocation#2966
allisterakun wants to merge 23 commits into
devfrom
semen_allocation

Conversation

@allisterakun

Copy link
Copy Markdown
Collaborator

Context

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

What

Why

How

Test plan

Input Changes

Output Changes

  • N/A

Filter

@github-actions

Copy link
Copy Markdown
Contributor

Current Coverage: %

Mypy errors on semen_allocation branch: 1229
Mypy errors on dev branch: 1180
49 more errors on semen_allocation branch

@github-actions

Copy link
Copy Markdown
Contributor

🚨 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.
🚨 Some tests have failed.

@github-actions

Copy link
Copy Markdown
Contributor

Current Coverage: %

Mypy errors on semen_allocation branch: 1233
Mypy errors on dev branch: 1180
53 more errors on semen_allocation branch

@github-actions

Copy link
Copy Markdown
Contributor

🚨 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.
🚨 Flake8 linting errors were found. Please fix the linting issues.
🚨 Some tests have failed.

@github-actions

Copy link
Copy Markdown
Contributor

Current Coverage: %

Mypy errors on semen_allocation branch: 1221
Mypy errors on dev branch: 1168
53 more errors on semen_allocation branch

@github-actions

Copy link
Copy Markdown
Contributor

🚨 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.
🚨 Flake8 linting errors were found. Please fix the linting issues.
🚨 Some tests have failed.

@github-actions

Copy link
Copy Markdown
Contributor

Current Coverage: %

Mypy errors on semen_allocation branch: 1221
Mypy errors on dev branch: 1168
53 more errors on semen_allocation branch

@github-actions

Copy link
Copy Markdown
Contributor

🚨 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.
🚨 Flake8 linting errors were found. Please fix the linting issues.
🚨 Some tests have failed.

@github-actions

Copy link
Copy Markdown
Contributor

Current Coverage: %

Mypy errors on semen_allocation branch: 1221
Mypy errors on dev branch: 1168
53 more errors on semen_allocation branch

@github-actions

Copy link
Copy Markdown
Contributor

🚨 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.
🚨 Flake8 linting errors were found. Please fix the linting issues.
🚨 Some tests have failed.

@github-actions

Copy link
Copy Markdown
Contributor

Current Coverage: %

Mypy errors on semen_allocation branch: 1221
Mypy errors on dev branch: 1168
53 more errors on semen_allocation branch

@github-actions

Copy link
Copy Markdown
Contributor

🚨 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.
🚨 Flake8 linting errors were found. Please fix the linting issues.
🚨 Some tests have failed.

@YijingGong

Copy link
Copy Markdown
Collaborator

Thanks for putting this together, Allister! The whole framework is here! I like the direction of making semen allocation percentile-based and keeping assign_semen_type() fairly generic. I did notice a few things that I think we should clean up before merging.

Main things I noticed

1. Ranking index calculation seems inconsistent

I think the individual animal ranking index and the population ranking list are currently being calculated with different formulas.

For the individual animal, _calculate_ranking_index() seems to use:

ranking_index = 0.318 * EBV_fat + 0.13 * EBV_protein

But the population list used for breeding around herd_manager.py:472 seems to use:

ranking_index_for_breeding = 0.71 * EBV_fat + 0.29 * EBV_protein

Then later, assign_semen_type() compares them directly:

np.array(population_ranking_index) <= animal_ranking_index

I think that comparison only works if both sides are calculated with the same formula/scale. Otherwise the percentile may be biased. In this case, since the individual value is on a smaller scale, I would expect animals to get artificially low percentiles, which could skew the semen allocation.

Could we make the ranking index calculation come from one shared source of truth? For example, the same helper function or the same configured weights could be used both for the animal-level value and for the population ranking list.


2. The gate for selective reproduction seems to have drifted

assign_semen_type() itself looks nicely method-agnostic: it just takes a population list and an animal scalar, then converts that into a percentile. I think that part is good, especially if we want to support non-genetic ranking methods later.

The issue is more around when we build/pass the ranking list.

From what I can tell, the ranking list is only built when all of these are true, around herd_manager.py:465-469:

simulate_genetics
and selective_repro_strategy
and ranking_method == "genetic"
and all eligible animals have ranking indexes

But on the consume side, _perform_ai seems to only check:

selective_repro_strategy

So there are some config combinations that can reach assign_semen_type(None, None) and then crash deep inside _perform_ai. For example:

selective_repro_strategy: true
ranking_method: null

or:

selective_repro_strategy: true
ranking_method: "genetic"
simulate_genetics: false

These eventually hit something like:

np.mean(np.array(None) <= None)

which gives a TypeError.

Could we do two things here?

  1. Add config validation up front:

    • if selective_repro_strategy=True, require a non-null ranking_method
    • if ranking_method=="genetic", require simulate_genetics=True
  2. Use one shared gate for both building and consuming the ranking list, so _perform_ai does not call percentile-based allocation unless the ranking list was actually built.

Also, for the genuinely empty-list case, we may want a defined fallback or a clear error. Right now np.mean([]) would become nan, and that could silently fall through to an unintended semen type.


3. Calf sex rate constants may need one source of truth

I noticed this in the animal constants:

BEEF_MALE_CALF_RATE = 0.1

I would guess this is a typo? the male calf rate is expected be closer to 0.5 if it's not sex-sorted.

I also saw that animal_config.py still has related values:

male_calf_rate_conventional_semen: float = 0.53
male_calf_rate_sexed_semen: float = 0.10
selective_repro_strategy: bool = False
ranking_method: str = "genetic"
genetic_selection_index_weights: dict[str, float] = {
    "fat": 0.71,
    "protein": 0.29,
}
heiferII_semen_allocation_proportions: dict[str, float] = {
    "sexed_dairy": 0.5,
    "conventional_dairy": 0.5,
    "beef": 0.0,
}
cow_semen_allocation_proportions: dict[str, float] = {
    "sexed_dairy": 0.2,
    "conventional_dairy": 0.5,
    "beef": 0.3,
}

I am not totally sure whether these config fields are still active or stale. If the new code now uses hardcoded constants while these config fields still exist, we may have multiple sources of truth.

Could we either remove the stale config fields or route these values through config consistently? I think this would make it clearer where calf sex rates, ranking weights, and semen allocation proportions are supposed to come from.


Side bug I found while debugging

I also saw this output:

Setting AI day for AnimalType.LAC_COW but animal is already pregnant
setting sex manually to female

This looks like a real lifecycle issue.

Together with Claude, we found for cow #12984, the sequence I traced was roughly:

  1. She entered the herd already pregnant from a real insemination.
  2. On import, her embryo sex was correctly assigned as Sex.MALE.
  3. After switching repro programs, she entered OvSynch even though she was already pregnant.
  4. OvSynch set a new AI day.
  5. A phantom AI later ran and failed conception.
  6. That failed AI set semen_type and embryo_sex to None.
  7. The original real pregnancy was still confirmed later.
  8. At calving, embryo_sex was None, so the code manually forced the calf sex to female, even though the original calf sex was male.

So again the problem is the sampling from the animal pool. Could we add a guard before synchronization setup and/or before _perform_ai so pregnant animals cannot be scheduled for or receive AI?

This affects calf sex outputs and hormone use/AI/semen stats, but I think it's not in this PR's scope. If it looks right to you, I can create an issue for this. And you can fix it in another smaller bug-fixing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Animal][Reproduction] Semen Allocation

2 participants