-
Notifications
You must be signed in to change notification settings - Fork 102
Dynamosa Algorithm #1399
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
base: master
Are you sure you want to change the base?
Dynamosa Algorithm #1399
Conversation
|
@francastagna unfortunately due to sickness, had to skip some work this week. and now rest of the week i ll be fully in meetings. i doubt i ll have any time to look at this PR this week. |
arcuri82
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @francastagna @jgaleotti thanks for this PR. it is more than 8k LOCs. unfortunately, instrumentation is a critical component of EM, and I would have to look into this PR in every details. i started, but this clearly would take me at least a day dedicated to it. not sure when i ll have a time for that. you can start to address the comments I gave so far, meanwhile
| /** | ||
| * Whether to enable Dynamosa graphs generation in the Java agent | ||
| */ | ||
| public Boolean enableDynamosaGraphs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instrumentation is independent from search algorithm. in other words, there should be no reference to Dynamosa here.
| /** | ||
| * Whether to write generated graphs (DOT/PNGs) to disk on the agent side | ||
| */ | ||
| public Boolean writeCfg; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this only for debugging purposes? if so, clarify it in the comment. also, i guess this would not work when running expeirments in parallel, as all will overwrite same file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, only for debugging porpuses.
Probably won't work when running experiments in parallel you are right.
| /** | ||
| * Incremental DynaMOSA control-dependence graphs discovered since the last handshake. | ||
| */ | ||
| public List<ControlDependenceGraphDto> dynamosaCdgs = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no ref to Dynamosa here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean I should rename the variable to something like 'cdgs' to avoid talking about the algorithm running in the core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
controlDependenceGraph. and yes, there should be no reference to dynamosa under client-java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acrnoyms like cfg are fine inside short-lived code blocks, but not for class fields
client-java/controller-api/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.evomaster</groupId> | ||
| <artifactId>evomaster-client-java-instrumentation-shared</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove. api module with DTOs should not have this kind of dependencies
| * Contains only the data needed by EvoMaster core: branch objective identifiers, | ||
| * the subset that are roots, and the parent→child relationships between them. | ||
| */ | ||
| public class ControlDependenceGraphDto implements Serializable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it is under instrumentation-shared, then it is not a DTOs. also, here there is plenty of logic that is part of a DTO. a DTO is a POJO, with just public fields.
| /* | ||
| * Adapted from the EvoSuite project (https://github.com/EvoSuite/evosuite) | ||
| * and modified for use in EvoMaster's Dynamosa module. | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case, docs/reused_code.md needs to be updated
| public DominatorTree(ControlFlowGraph<V> cfg) { | ||
| super(DefaultEdge.class); | ||
|
|
||
| SimpleLogger.debug("Computing DominatorTree for " + cfg.getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no concatentation in debug logs. this applies to all instances in this PR. i ll not add more comments about it
|
|
||
| computeDominatorFrontiers(rootNode); | ||
|
|
||
| // toDot(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, I'll remove this
|
|
||
| createDominatorNodes(); | ||
|
|
||
| V root = cfg.determineEntryPoint(); // TODO change to getEntryPoint() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this TODO still relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not, i'll remove it.
|
|
||
| private void computeSemiDominators() { | ||
|
|
||
| for (int i = nodeCount; i >= 2; i--) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why i >= 2? add some explanation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We skip node 1 (i >= 2) because node 1 is the root/entry point.
I'll add a comment here.
Hi @arcuri82, thanks for taking a look at the PR and for the initial feedback. I’ll start addressing the comments you’ve provided so far. |
ab6b218 to
a44bc04
Compare
jgaleotti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please @francastagna address these changes plus those asked by @arcuri82
| pool.branchCounter = 0; | ||
| } | ||
| } | ||
| // fill the pool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this code incomplete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it was an old comment, I'll remove it!
|
|
||
|
|
||
| public static boolean isGraphsEnabled() { | ||
| System.out.println("enableGraphs: " + enableGraphs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why System.out is used? Shouldn't you be using the logging infraestructure for this?
| } | ||
| } catch (IOException e) { | ||
| // TODO Auto-generated catch block | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look correct, missing the exception here.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.evomaster</groupId> | ||
| <artifactId>evomaster-client-java-controller-api</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this new dependency necessary? why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! The instrumentation module uses DTOs from controller-api to exchange control dependence graph information with the controller.
Specifically:
GraphPool.java (lines 17-19) imports ControlDependenceGraphDto, BranchObjectiveDto, and DependencyEdgeDto
These DTOs are used in GraphPool.buildControlDependenceDto() (lines 344-418) to build the graph representation that gets exported to the controller
| * This controls only the persistence of graphs to disk; graph creation is controlled separately. | ||
| */ | ||
| @Cfg("Enable writing CFG/CDG graphs to disk on the agent side") | ||
| var writeCfg: Boolean = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature should be experimental and false by default
docs/options.md
Outdated
| |`useResponseDataPool`| __Boolean__. Enable the collection of response data, to feed new individuals based on field names matching. *Default value*: `true`.| | ||
| |`useTimeInFeedbackSampling`| __Boolean__. Whether to use timestamp info on the execution time of the tests for sampling (e.g., to reward the quickest ones). *Default value*: `true`.| | ||
| |`weightBasedMutationRate`| __Boolean__. Whether to enable a weight-based mutation rate. *Default value*: `true`.| | ||
| |`writeCfg`| __Boolean__. Enable writing CFG/CDG graphs to disk on the agent side. *Default value*: `true`.| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default value should be false as this feature should only be turned on when using DYNAMOSA
…ntrolDependenceGraphConfig
jgaleotti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No further changes from my side. @arcuri82 can proceed with his own review
PR Summary
We implemented the DynaMOSA many‑objective search algorithm, wired it into EvoMaster core, and added a graph‑based instrumentation pipeline that builds control dependence graphs (CDGs) and exposes them to the algorithm via a lightweight DTO and remote protocol.
Pseudocode:
Source:
https://www.researchgate.net/publication/313164878_Automated_Test_Case_Generation_as_a_Many-Objective_Optimisation_Problem_with_Dynamic_Selection_of_the_TargetsMain Changes
New DynaMOSA algorithm (
DynaMosaAlgorithm)Branch dependency graph and multi‑criteria manager
BranchDependencyGraphbuilds a lightweight DAG of branch objectives fromControlDependenceGraphDto(roots, children, and all objectives).MulticriteriaManagerkeeps track of covered/uncovered branch targets and computes the current focus set by combining CDG roots with uncovered children; falls back to all uncovered targets when needed.Instrumentation: CFG/CDG construction and export
EvoMasterGraph, CFG and CDG classes,GraphPool) that computes per‑method CDGs from bytecode.ControlDependenceGraphDto(objectives, root ids, edges) as the minimal, serialized view of a CDG sent from the agent to core.DynamosaConfigflags and controller hooks (InstrumentationController,AgentController) to enable/disable graph construction and to stream CDG snapshots incrementally.Core / driver wiring
EMConfigandMainsoAlgorithm.DYNAMOSAis available and correctly instantiated for REST, GraphQL, RPC, and web‑frontend problems.ControlDependenceGraphDtoobjects used to maintain the branch dependency graph in core.