Refine variance mappings, colony indexing, and brood summaries by gregorgorjanc · Pull Request #2 · janaobsteter/SIMplyBee_fork

gregorgorjanc · 2026-09-30T09:45:25Z

This updates variance mappings and colony indexing together, preserving the changes on Jana’s Future branch.

Colony IDs are consistently character IDs and remain independent of queen IDs when queens are removed or replaced. Numeric indexing selects positions; character indexing selects colony IDs. Crossing plans support non-numeric donor-colony IDs and reordered colony names, with regression tests for missing entries, empty collections, subsetting, and replacement.

The forward and inverse variance mappings distinguish within-bee covariance from queen–worker covariance, support explicit colony design coefficients and random or balanced worker allocation, validate inputs, and handle zero variance. Both mappings return h2Queen, h2Worker, h2WorkerGroup, and h2Colony. Brood summaries handle queenless colonies and empty populations consistently; simulation-parameter forwarding and splitPColonyStrength() are corrected. Updated help, vignettes, shared references, and release notes accompany these changes.

The consolidated agent guidance replaces the duplicate agent file. Build exclusions cover development material, including AGENTS.md and TestingVarianceComponents.R. future is moved from Imports to Suggests for vignette use; package functions import future.apply, removing the unused-import NOTE.

Validation of the consolidated tree committed as 59a15c4:

Full devtools::check(document = FALSE), including vignette rebuilds: 0 errors, 0 warnings, 0 notes.
Full package tests: 852 passing assertions, no failures; one pre-existing metadata warning in the getCaste test. The installed testthat also reports being built under a newer R patch version.
Coverage: 51.26%.
git diff --check: clean.
Air reports existing formatting differences on both parent branches. Jarl reports lint findings and crashes on some inputs; broad formatting changes were not included.
Documentation was regenerated with installed roxygen2 7.3.3; unrelated generator-version churn was reverted. The recorded version is 8.1.0.
F2 vignettes are excluded from package builds and were not separately rendered for this update.

gregorgorjanc · 2026-09-30T19:42:05Z

Ok, tests on GitHub finally pass;)

gregorgorjanc · 2026-10-03T17:16:18Z

@janaobsteter I noticed that we were not consistent with colony ID - it was integer/numeric, whereas in AlphaSimR we have them character so they behave as names. This is very important for indexing (see expanded tests).

ALSO, I don't know why you changed colony ID to queen ID. This is not the same thing. Colony is an entity that holds the queen, so if we use queen ID for the colony ID, we would then have just one ID (of the queen) and be done with it. But, we can have a colony without the queen, so the colony would then also be without the ID, which does not make sense, since we can still have such a colony at some point in time. I know this is PITA, but being pedantic with such bits really is important or you can't reason safely about what an ID means and what does it represent.

I feel that both of these points could have been baked in some of your scripts that you have developed - would be good to check and fix this in the scripts.

I will run some more local tests to check if we conflated the two anywhere.

gregorgorjanc · 2026-10-03T17:23:01Z

Yeah, this colony/queen ID assumption is already causing issues/bugs - see HighlanderLab#644

janaobsteter

Leaving some comments below

janaobsteter · 2026-10-06T11:37:52Z

+#'   so it contributes one missing ID and preserves alignment with the MultiColony's entries.
+#'   An empty Pop or MultiColony contains no individuals or entries to identify.
+#'   An empty colony created with \code{createColony()} still has an assigned ID;
+#'   an uninitialised \code{new("Colony")} has no assigned ID.


MultiColony would only have IDs if create with TRUE for
@PARAM populateColonies boolean, whether to create n empty Colony objects within with assigned ID
#'

I think we should discuss the colony vs queen ID. I think the current logic is broken the way we have it implemented. I think we started with colony ID (=ID of the colony as a physical entity) and queen ID (=ID of the queen leading the colony). I am fully aware that these two IDs are used interchangeably, however I think this is bad from data management perspective since a colony can be without a queen for a while, so we need to have an ID of that empty entity and without a queen it won't have one if there is no queen. An option would be for the getId(colony) to just call getId(colony@queen) and drop colony IDs altogether, but I still don't understand what one would use for colony ID that does not have the queen and some functions like getId could return NA/NULL which becomes PITA to handle in scripting. Happy to discuss and drop these colony/queen IDs from the PR (I tried to do to many things). Thoughts?

We started off without colony IDs, then realised we need them. I can't find in the issues exactly why but part of the issue was that two colonies could have the same queen, and also, when the queen changes, the offspring in the hive is still from the previous queen, so need to track the hive separately. However, in the simulation, we would track the queen since we are tracking the genetics. I suggest we keep the colonyIDs for now (separate from queen IDs)

FYI Now we do have a warning that a queen is already in another colony, I think, I got a warning to this effect when I was doing polishing of this PR tests.

OK, I am glad to hear that we are converging on keeping hive/colony ID and queen ID as separate. I think the challenge will now be to ensure this works well and as intended. We have an issue at HighlanderLab#644 for this, so I think we just need to ensure that current code/PR is as it should and then we systematically address the HighlanderLab#644 issue.

janaobsteter · 2026-10-06T11:39:42Z

-
-  if (!workersFUN %in% c("sum", "mean")) {
-    stop("Invalid value for workersFUN. We currently only support either 'sum' or 'mean'.")
+#' Calculate coefficients to map across individual and colony variances


This all is a bit long for function description, can we shorten?

Hmm, I agree it's long, and I genuinely spent a lot of time trying to cut down on the words, while still communicating what is what. Can you suggest what to cut?

janaobsteter · 2026-10-06T12:10:14Z

 #'
 #' @description Level 2 function that re-queens a Colony or
 #'   MultiColony object by adding a mated or a virgin queen, removing the
-#'   previous queen, and changing the colony id to the new mated queen.


The code actually wasn't doing this

Indeed! So, where do we then assign the new ID to colony then? Looks like we caught a bug here - do you agree? - we initially had colony and queen IDs separate, then I think you were fed up by this and started migration to colony ID = queen ID, but did not implemented that across all the package? Is that a fair summary. Let's discuss how to manage this.

janaobsteter · 2026-10-06T12:15:15Z

    if (isColony(x)) {
      swarmColony <- createColony(x = x@queen, simParamBee = simParamBee)
-      # It's not re-queening, but the function also sets the colony id
+      # The swarm receives a new colony ID, independent of its queen ID


Re this and the comment in the discussion - I think we were discussing whether the colony ID tracks he physical hive or the genetics. But the genetics is tracked with the queen's ID

I think this is more complicated and my worry is more about indexing when queen is missing and such edge cases. If we want that colony ID = queen ID then we just make the getId(colony) call getId(colony@queen), but that can also return NA/NULL which are annoying to handle. Let's discuss or maybe pull out from this PR.

janaobsteter · 2026-10-06T12:21:03Z

+
 ## Bug fixes

+- Moved `future` to Suggests because it is used by the parallelisation vignette;


This was not "moved" since it is not present in the previous version

I have this on my end:

Are we dropping it, moving it elsewhere, or something different?

Co-authored-by: Gregor Gorjanc <gregor.gorjanc@gmail.com>

gregorgorjanc · 2026-10-06T20:26:58Z

@janaobsteter I think I went over all your comments - good critical stuff! ;) See my replies and suggest how we should move onwards.

gregorgorjanc · 2026-10-07T17:47:58Z

@janaobsteter I did some security checking of the code and Codex suggested changes in gregorgorjanc@e00f9d3 (after my review/polish). I think the nParalleleWorkers and nThreads bit can be quite important (and confusing!) detail. It also suggested some args handling in the passed functions - I only did one and left the rest for future so this PR doesn't digress (it already is!): HighlanderLab#646

janaobsteter · 2026-10-12T09:05:06Z

OK, then let's leave it in, it's also easier to correct latter

janaobsteter · 2026-10-12T09:11:36Z

@gregorgorjanc , if you just move future back to imports and change the text above (if you see any merit), then I will merge this PR and let's move on. I think colony ID handling should be ok, with the exception of that case in swarm where it overwrote it with the queenID (not sure what happened there).

gregorgorjanc and others added 8 commits September 30, 2026 10:40

WIP

c1a9b82

Update documentation

662047d

Exclude AGENTS.md from R package builds

c7ba958

Added h2 to mapping output

6c4d9df

Docs

946a8f0

Document heritability outputs in release notes

8d83dd5

Update documentation

69aa910

Polish of various things (pHomBrood, etc.)

605301d

gregorgorjanc changed the title ~~Refine individual and colony variance mappings and documentation~~ Refine variance mappings and fix brood summaries and parameter forwarding Sep 30, 2026

gregorgorjanc and others added 2 commits September 30, 2026 17:58

Update documentation

ebb8e8a

Ignore local IDE state and generated build artifacts

72b4311

gregorgorjanc mentioned this pull request Sep 30, 2026

Colony-level variances to individual-level variances HighlanderLab/SIMplyBee#49

Open

Fortifying the pHomBrood test

dd323de

gregorgorjanc added 3 commits October 1, 2026 08:20

more testing

1d6010b

Polish indexing, character IDs, colony vs queen ID, and AGENTS.md

348249d

Merging different worktrees

59a15c4

gregorgorjanc changed the title ~~Refine variance mappings and fix brood summaries and parameter forwarding~~ Refine variance mappings, colony indexing, and brood summaries Oct 3, 2026

Update documentation

90873c1

gregorgorjanc mentioned this pull request Oct 3, 2026

Colony ID bookkeeping and queen-to-colony mapping in cross() HighlanderLab/SIMplyBee#644

Open

janaobsteter reviewed Oct 6, 2026

View reviewed changes

gregorgorjanc commented Oct 6, 2026

View reviewed changes

Comment thread R/Functions_L0_auxilary.R Resolved

gregorgorjanc and others added 2 commits October 6, 2026 20:43

Apply suggestion from @gregorgorjanc

25f3b38

Update documentation

ecc27d2

gregorgorjanc mentioned this pull request Oct 6, 2026

Document allowed NULL input in functions? HighlanderLab/SIMplyBee#645

Open

Repository owner deleted a comment from janaobsteter Oct 6, 2026

Fix empty else block in Functions_L3_Colonies.R

5cfaa2c

gregorgorjanc commented Oct 6, 2026

View reviewed changes

Comment thread NEWS.md Resolved

gregorgorjanc commented Oct 6, 2026

View reviewed changes

Comment thread NEWS.md Resolved

Apply batched suggestions from code review

98aaa9b

Co-authored-by: Gregor Gorjanc <gregor.gorjanc@gmail.com>

gregorgorjanc and others added 2 commits October 7, 2026 18:43

Fix colony callback dispatch and clarify parallelisation

e00f9d3

Update documentation

ab45fc8

janaobsteter marked this pull request as ready for review October 26, 2026 11:27

janaobsteter merged commit 299b231 into janaobsteter:Future Oct 26, 2026
5 of 6 checks passed


		## Bug fixes

		- Moved `future` to Suggests because it is used by the parallelisation vignette;

Conversation

gregorgorjanc commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gregorgorjanc commented Sep 30, 2026

Uh oh!

gregorgorjanc commented Oct 3, 2026

Uh oh!

gregorgorjanc commented Oct 3, 2026

Uh oh!

janaobsteter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gregorgorjanc commented Oct 6, 2026

Uh oh!

gregorgorjanc commented Oct 7, 2026

Uh oh!

janaobsteter commented Oct 12, 2026

Uh oh!

janaobsteter commented Oct 12, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gregorgorjanc commented Sep 30, 2026 •

edited

Loading