Stress escapees refactor - #328
Conversation
| // TODO - add any signals? | ||
| const size_t parent_pop_idx = parent_pos.GetPopID(); | ||
| emp::Ptr<Organism> parent = this->GetOrgPtr(parent_pop_idx)->GetSymbionts()[parent_pos.GetIndex() - 1]; | ||
| emp_assert(!parent->IsHost()); |
There was a problem hiding this comment.
I'm confused where "parent" is being defined now
There was a problem hiding this comment.
your right! that is pretty confusing. I wonder why that passed compile
There was a problem hiding this comment.
changed to sym_parent_ptr
| std::function<std::optional<emp::WorldPosition>( | ||
| size_t, /* Parent's host location id in world (pops[0][id])*/ | ||
| emp::Ptr<sgp_sym_t> /* Pointer to symbiont parent (producing the sym offspring) */ | ||
| emp::Ptr<sgp_sym_t> sym_offspring_ptr, |
There was a problem hiding this comment.
I don't think that you can include parameter names, hence the previous syntanx
There was a problem hiding this comment.
Also, why are we now passing the offspring as well as the parent? We should only need whatever symbiont is being used to check for compatibility
There was a problem hiding this comment.
Removed parameter names. Good question with the offspring; They are not used currently and could easily be removed. we are checking the task profile of the parent with the prospective host, but that doesent quite sit right with me. We are determining the "fate" of the offspring based on the parent, not the symbiont itself. In other words fun_horizontal_transmission_compatibility_check determines the compatibility of the parent and prospective host, but that value is used to determine the compatabilty of the sym and prospective host.
There was a problem hiding this comment.
See SGPWorld.test.cc comment as well
There was a problem hiding this comment.
Let's focus on just refactoring to capture existing functionality and not changing functionality in this PR, since otherwise too many things are changing and it is likely that a bug gets introduced.
The parent is injecting its offspring into a new host and the offspring hasn't done anything yet, so it can't do its own infecting. The fate of offspring is very often dependent on the parent's behavior ;)
|
|
||
| emp::Ptr<sgp_host_t> host = emp::NewPtr<sgp_host_t>(&random, &world, &config); | ||
| emp::Ptr<sgp_sym_t> symbiont = emp::NewPtr<sgp_sym_t>(&random, &world, &config); | ||
| symbiont->GetHardware().GetCPUState().MarkTaskPerformed(8); |
There was a problem hiding this comment.
Why was this necessary to add?
There was a problem hiding this comment.
I moved it out of the WHEN block below. Initially in my refactor I had:
void SGPWorld::SetupFindHostForHorizontalTransmission() {
...
const bool compatible = fun_horizontal_transmission_compatibility_check(
*prospective_host_ptr,
*sym_offspring_ptr
);
...
}
Now:
void SGPWorld::SetupFindHostForHorizontalTransmission() {
...
const bool compatible = fun_horizontal_transmission_compatibility_check(
*prospective_host_ptr,
*sym_parent_ptr //This Line Changed
);
...
}
As mentioned earlier in your comment in SGPWorld.h, initially I thought it would make sense to compare the offspring to the host. Plus, I saw this in Reproduce():
offspring_cpu_state.SetParentTasksPerformed(
hardware.GetCPUState().GetTasksPerformed()
);
offspring_cpu_state.SetParentFirstTaskPerformed(
hardware.GetCPUState().GetFirstTaskPerformed()
);
By putting MarkTaskPerformed() before Reproduce() in the test, I thought it would properly get copied to the offspring, and then the offspring would be able to pass the compatibility check. When I was wrong, I forgot to move this line back and remove the reproduction.
| symbiont->GetHardware().GetCPUState().MarkTaskPerformed(8); | ||
| WHEN("Task matching is not required for horizontal transmission") { | ||
| auto pos_found = world.FindHostForHorizontalTrans(source_id, symbiont); | ||
| auto pos_found = world.FindHostForHorizontalTrans(offspring, symbiont, symbiont->GetLocation()); |
There was a problem hiding this comment.
Pretty sure it should be just "symbiont" being passed since offspring has no useful information
| world.AddOrgAt(host, 0); | ||
| size_t source_id = symbiont->GetLocation().GetPopID(); | ||
| emp::Ptr<Organism> offspring_org = symbiont->Reproduce(); | ||
| emp::Ptr<sgp_sym_t> offspring = static_cast<sgp_sym_t*>(offspring_org.Raw()); |
There was a problem hiding this comment.
You'll be able to remove these lines once you make the change to not pass offspring, so please don't forget to do so
anyaevostinar
left a comment
There was a problem hiding this comment.
Some things to change back. Make sure to be careful not to change functionality other than the specific thing that is the target. Also, it's good to clean up small things, but try to resist moving small lines around when they really don't change much since it adds to the complication of reviews and merge conflicts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #328 +/- ##
==========================================
- Coverage 77.09% 77.09% -0.01%
==========================================
Files 108 108
Lines 23155 23131 -24
Branches 1615 1613 -2
==========================================
- Hits 17852 17832 -20
+ Misses 5232 5229 -3
+ Partials 71 70 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Overview:
Stress References removed from SGPWorld* and into SGPW_InteractionMechanismSetup.cc
SymDoBirth modified to take offspring, parent, and world position
SetupHorizontalTransmissionCompatibilityMode() changed to set one functor
Other:
Next steps: