Fixing parallelization issue

[Klemet]

Fixing an issue that could cause rare errors, where several site variables weren't properly locked, resulting in thread workers conflicting with each other and crashing the simulation (see #25).

Context: In ExtensionBase.ReproduceCohorts, each parallel thread owns exactly one site, and every cohort write (AddNewCohort, planting.TryAt) targets the writing thread's own site. This means each site's cohort list has exactly one writer — its owning thread. But there can still be conflict between parallel threads when one thread edits the cohorts in its site while another parallel thread is reading these cohorts as a neighboring site of their own site, leading to a crash (see #25).

What this PR does: Adds locking around cohort reads of a neighbour's site, and confirms writes are already protected by the site's lock (to exclude concurrent neighbour-readers).

Reasoning for the locks:

• Neighbour reads (MaturePresent(species, neighbor)) must take the neighbour's lock to read safely — this is the actual fix.
• Writes already take the site's lock — nothing to change.
• Own-site reads (SufficientResources(species, site), Establish(species, site), on-site MaturePresent) need no lock, since the only writer is the calling thread.

Fix made with OpenCode, reviewed manually and tested on the same scenario (and same number of threads) which produced an error as described in #25 . The error is now gone.

[Klemet]

As recommended by @achubaty, I've added a "sub-version" stamp in the final dll so that they can be distinguished. The Assembly version remains 10.0 to avoid compatibility issues. It's just the file version and product version that change.

[achubaty]

@Klemet it looks like your updated ProductVersion use a short-sha (31337e2) then a period (.) and the long form of the same sha (31337e2de21d796154...), whereas the previous one used only the long form of the sha.

[achubaty]

Thanks for tracking this down, Clément. I'm looking at these changes as a downstream user, not a maintainer/owner, so please take everything below as input rather than gatekeeping. I used Claude Code to help work through the diagnosis and review, so the analysis below is a mix of my own reading and its output. I've sanity-checked the reasoning and the suggested code, but extra scrutiny is warranted, especially on the concurrency arguments.

Your diagnosis looks right to me, and the core of the fix looks right too. I have a few observations that might be worth folding in, plus one idea for later.

One framing that might help reviewers (and might be worth a line in the PR body), since it's what decides which locks are load-bearing:

ExtensionBase.ReproduceCohorts hands each thread one site, and every cohort write (AddNewCohort, planting.TryAt) targets the writing thread's own site. So as far as I can tell, each site's cohort list has exactly one writer — its owning thread. If that holds, then:

• Reads of a neighbour's cohorts (MaturePresent(species, neighbor)) need the neighbour's lock — which is what this PR does, and I think that's the actual fix.
• Writes need the site's lock to exclude concurrent neighbour-readers — also done.
• Reads of the site's own cohorts (SufficientResources(species, site), Establish(species, site), on-site MaturePresent) shouldn't need a lock, since the only writer is the calling thread. That's why I think the unlocked calls at WardSeedDispersal.cs:31 and :39 are fine as-is, and why the new on-site lock may be removable.

Worth stating explicitly either way, so a future contributor doesn't "fix" those unlocked calls without realizing they're intentional.

On coverage — this looks complete to me, but it needs a maintainer's eyes rather than mine. For whatever it's worth, I had a look at the alternatives: NoDispersal and UniversalDispersal read own-site only; the MaturePresent sweeps in density-seeding/Algorithm.cs:118 and demographic-seeding/Algorithm.cs:393 run in serial landscape-wide loops rather than the per-site Parallel.For; and the other Parallel.For (ExtensionBase.cs:188, AgeCohorts) is own-site only. So WardSeedDispersal seems to be the only exposed path — though someone with more history in this code should confirm I haven't missed a caller.

An idea for later, definitely not for this PR: the neighbour lock now sits in the innermost dispersal loop, so long-dispersal species with a large MaxSeedQuarterNeighborhood will pay a fair amount of uncontended Monitor.Enter/Exit. If that ever shows up in profiling, a per-timestep mature-presence snapshot taken before the parallel loop and read lock-free might be cheaper. I think that would be semantically equivalent, since cohorts added during reproduction are age 0–1 and below sexual maturity for any realistic parameterization — so a concurrent AddNewCohort shouldn't be able to flip MaturePresent's logical answer, only corrupt the enumeration. Would want someone who knows the science better than me to confirm that. Incidentally, the same reasoning implies the current parallel path is already non-reproducible run-to-run (whether a neighbour's mid-loop mutation is observed depends on thread timing), so a snapshot might buy determinism as a side benefit.

Seeding.cs: nothing from me, that hunk reads correctly.

On testing: it might be worth pulling the "reproduced #25's crash on the same scenario and thread count, confirmed gone" detail up into the PR body more prominently. With no test suite backing this, that reproduction is the strongest evidence available, and it's currently easy to miss.

[Klemet]

Thanks a lot for taking the time to take a look, Alex ! I'm reassured to have your opinion, and I've implemented all of the tweaks you've proposed 😁.

I'm going to try the updated library one more time - this is going to take time, since the only simulation where I had the bug is the same as in #25, and it's a big and long one. That should take a couple of days at the least. In the meanwhile, other reviewers/maintainers can take a look. I think we're close to a solid fix now.

[Klemet]

I finished re-testing this version that contained the edits proposed by Alex.

Everything worked perfectly; and the stripping procedure that replaced the use of one lock per site reduced memory usage by around 300Mb for my large landscape, as planned !

Everything looks good for me; if you have any more questions or remarks before a merge, feel free to ask !