Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in systematics #419

Closed
wants to merge 11 commits into from
Closed

Fix bug in systematics #419

wants to merge 11 commits into from

Conversation

emilydolson
Copy link
Collaborator

@emilydolson emilydolson commented Mar 8, 2021

This pull request:

  • Fixes the bug @amlalejini identified where the MRCA does not get updated if it is alive and then dies at a time when it dying should cause the MRCA to change.
  • Incorporates @abbywlsn's updates to systematics documentation, tests, and features from her summer WAVES project
  • Fixes Update DataFile tests #397 by removing floating point numbers from the file comparison test and making a separate floating point test (I know it's not technically systematics, but it was broken by the last systematics update)

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #419 (b6bbde0) into master (87a9b3c) will increase coverage by 0.40%.
The diff coverage is 98.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #419      +/-   ##
==========================================
+ Coverage   82.71%   83.11%   +0.40%     
==========================================
  Files         297      298       +1     
  Lines       37060    37692     +632     
==========================================
+ Hits        30654    31328     +674     
+ Misses       6406     6364      -42     
Impacted Files Coverage Δ
include/emp/Evolve/Systematics.hpp 97.73% <97.67%> (+4.35%) ⬆️
tests/Evolve/Systematics.cpp 99.09% <98.15%> (-0.91%) ⬇️
include/emp/Evolve/SystematicsAnalysis.hpp 100.00% <100.00%> (ø)
include/emp/Evolve/World.hpp 96.37% <100.00%> (+0.01%) ⬆️
tests/Evolve/Systematics_phylodiversity.cpp 100.00% <100.00%> (ø)
tests/data/DataFile.cpp 98.50% <100.00%> (+8.50%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87a9b3c...b6bbde0. Read the comment docs.

@emilydolson
Copy link
Collaborator Author

emilydolson commented Mar 11, 2021

Okay, this is ready for review. I know the diff looks scary, but that's mostly just because I rearranged Systematics.hpp so that the methods weren't randomly scattered across the file. The only substantive changes in this PR are:

  • Adding/improving tests
  • Documentation
  • Fixing the bug mentioned in the description
  • Adding the very start of some systematics normalization tools
  • Removing the time-related arguments to AddOrg and RemoveOrg in the systematics-manager. They made it too easy to accidentally call the wrong version of the function and are unnecessary now that systematics managers track time internally.
  • Fixing a few places where the systematics manager was still using ints instead of world positions.
  • Subtly change the order of events in world::Update() so that the systematics manager is updated before on placement signals are triggered for synchronous worlds.

@emilydolson
Copy link
Collaborator Author

Superseded by #451

@emilydolson emilydolson closed this Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update DataFile tests
2 participants