Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts the simulator’s result logging to better align input snapshots with communication-step timestamps, addressing Issue #79 where result files could end up with incorrect/undesired time indexing/row alignment.
Changes:
- Always logs inputs at communication-step boundaries (in addition to logging when inputs change) to keep result rows consistent.
- Minor comment formatting cleanup in the simulation loop.
| current_time = self.env.time | ||
| # Track if a communication step is reached. | ||
| # At communication times, the inputs are always saved | ||
| full_comm_step = ((current_time - self._last_communication_time) == 0) |
There was a problem hiding this comment.
full_comm_step relies on exact float equality ((current_time - self._last_communication_time) == 0). With env.time advancing via floating-point additions, this can fail due to rounding and prevent inputs from being logged at communication boundaries (leaving the most recent output row’s inputs as None). Consider using a tolerant comparison (e.g., math.isclose with an abs_tol) or a step-index based check (e.g., i == 0 for the first sub-step of each communication interval).
| current_time = self.env.time | |
| # Track if a communication step is reached. | |
| # At communication times, the inputs are always saved | |
| full_comm_step = ((current_time - self._last_communication_time) == 0) | |
| # Track if this is the first simulation sub-step within the current | |
| # communication interval. At communication boundaries, the inputs are | |
| # always saved to ensure they are associated with the correct outputs. | |
| full_comm_step = (i == 0) |
Closes #79