Conversation
heavenfall
commented
Oct 17, 2025
- Created new log featue.
- Redone listeners, allowing observables on currently unidirectional_search.
- Framework for observables to make writing to streams easier.
- Added support for posthoc through observables, included a default grid trace.
ae1d43b to
141532f
Compare
There was a problem hiding this comment.
As discussed, not sure what the purpose of this object is. Suggest we jettison and rely on users creating/managing their own ostream objects.
There was a problem hiding this comment.
updated the stream_observer to be simple like a wrapper, jettisoned most of the opening stream and removed the shared_stream feature. It is now mainly an interface with a stream pointer and define the concept of if setup to stream used in observer objects.
Added a line_observer object that instead of output to stream, it supports writing line-by-line. The user can choose to open a stream or attach to a log_sink.
dharabor
left a comment
There was a problem hiding this comment.
Thanks for this Ryan. I think we need a few small refactorings before merging.
include/warthog/io/posthoc_trace.h
Outdated
| print_posthoc_header(); | ||
|
|
||
| int | ||
| search_id() const noexcept |
There was a problem hiding this comment.
Posthoc should not know about internals of search. Based on discussion, suggest to replace with an ::open and ::close function.
There was a problem hiding this comment.
Done. The updated posthoc_trace now establishes the print_posthoc_header function, which will trigger with a call to open(). search_id is no longer used, instead it is dependent on the user calling open(stream) to begin listening, and close() to end listening.
| { | ||
| initialise_node_(n, current->get_id(), gval, pi, par, sol); | ||
| if(n->get_f() < sol->sum_of_edge_costs_) | ||
| if(n->get_f() <= sol->sum_of_edge_costs_) |
There was a problem hiding this comment.
Why do we want weakly dominated nodes? Should this be strictly less than?
There was a problem hiding this comment.
reverted change
| { | ||
|
|
||
| void | ||
| grid_trace::print_posthoc_header() |
There was a problem hiding this comment.
This implementation is not in a file which corresponds to its associated header. Let's keep implementations filenames consistent with their corresponding headers.
apps/warthog.cpp
Outdated
| listener_grid& l = std::get<listener_grid>(algo.get_listeners()); | ||
| l.stream_open(trace_file); | ||
| l.search_id(0); | ||
| l.set_grid(expander->get_map()); |
There was a problem hiding this comment.
Why does the listener API need to reflect details about the solver implementation? Suggest to refactor for the same reasons as search_id.
There was a problem hiding this comment.
interface updated
include/warthog/io/observer.h
Outdated
|
|
||
| // io/observer.h | ||
| // | ||
| // The observer pattern defines methods to tightly-bind user provided observers |
There was a problem hiding this comment.
Please add a definition of observers. What are they and why do we need them?
| // | ||
| // Logging utility. | ||
| // | ||
| // The log_sink is a non-owning copyable struct that points to the data and |
There was a problem hiding this comment.
Please give a general description of the logging facility and its capabilities.
There was a problem hiding this comment.
added extra high-level description