Adding simulation reduction rules #487
Conversation
samo538
force-pushed
the
sim_rt
branch
2 times, most recently
from
e94a0da to
b4c7441
Compare
Adda0
left a comment
Adda0
left a comment
There was a problem hiding this comment.
Thank you for the PR. Looks good to me overall.
| nfa/delta.cc | ||
| nfa/operations.cc | ||
| nfa/builder.cc | ||
| nfa/reductionSimulation.cc |
There was a problem hiding this comment.
| nfa/reductionSimulation.cc | |
| nfa/reduction-simulation.cc |
| /** | ||
| * @brief TODO | ||
| */ | ||
| Nfa reduce_size_by_simulation(const Nfa& aut, StateRenaming &state_renaming, const std::string& simulation_direction); | ||
|
|
||
| /** | ||
| * @brief TODO | ||
| */ | ||
| Simlib::Util::BinaryRelation compute_direct_simulation(const Nfa& aut); |
| using namespace mata::nfa; | ||
|
|
||
| namespace { | ||
| // Merging based on the first and second rule (From the paper 'On NFA Reduction') |
|
|
||
| namespace { | ||
| // Merging based on the first and second rule (From the paper 'On NFA Reduction') | ||
| Nfa merge_in_one_direction(const Nfa& aut, StateRenaming& state_renaming, |
There was a problem hiding this comment.
This function should be better documented. Especially, the little_brother parameter should be explained.
| // Build the resulting automaton based on the simulated_states and state_renaming | ||
| for (State p = 0; p < num_of_states; ++p) { | ||
| // If the state is simulated, its not added to the resulting automaton | ||
| if (std::find(simulated_states.begin(), simulated_states.end(), p) != simulated_states.end()) { |
There was a problem hiding this comment.
Is there a method simulated_states.find() to use instead of the general std::find()?
| nfa/delta.cc | ||
| nfa/operations.cc | ||
| nfa/builder.cc | ||
| nfa/reductionSimulation.cc |
There was a problem hiding this comment.
The indentation seems wrong.
There was a problem hiding this comment.
If you used algorithms from some papers, could you add links to those? Or if there is any write-up about the approach chosen here, it would be also beneficial. If there is not, maybe just a general description in the function definition explaining what the algorithm does would go a long way to improving the readability of the introduced changes.
|
@jurajsic Could you also have a look before July 14? We want to merge the PR this month, ideally. |
There was a problem hiding this comment.
Could you rename this file to reduce.cc and move here all reduction algorithms? It might nicely clean up the main catch-all files.
|
Are there any benchmarks comparing the currently implemented reduction in |
3 participants
Project practice 2 simulation enhancement