Skip to content

Finish the truncation change: the log, the final frame, the README and the changelog - #104

Merged
thc1006 merged 2 commits into
developfrom
fix/truncation-log-and-final-frame
Jul 28, 2026
Merged

thc1006 merged 2 commits into
developfrom
fix/truncation-log-and-final-frame

Conversation

@thc1006

@thc1006 thc1006 commented Jul 28, 2026 •

Copy link
Copy Markdown
Member

Finishes the split between terminated and truncated. That split was right; several places still read terminated from when that one flag also covered the clock, and none of them moved with it.

I swept for all of them rather than fixing the two that were reported.

The environment

if _timeout:
    logger.info("Terminated: Reached max time")

Running out of horizon is truncation now, and this line is the one thing an operator reads to find out why a run stopped.

if _remainder == 0 or terminated:
    self._render_frame()

Frames are drawn every 0.1 s of simulation time, or when the episode ends. While the clock counted as termination, an episode running out of horizon always drew its final frame. Afterwards the gate only fires on terminated, so a truncated episode draws one only if its last step happens to land on the cadence. Scenario 0 truncates at step 9999 against a cadence of 10, so it does not. Scenario 1 ends this way normally.

The README, which is where a competitor writes their loop from

It described env.step() as returning "the new observations, reward, termination flag, and additional info", which is four values and one flag, and said the episode ends when the time limit is reached or the rocket hits the ground, as though those were one outcome. It now names both flags, says which cause each is, and shows the loop.

The changelog

Had no entry for any of this. Added under Changed as breaking for every agent loop, alongside the observation copies from #99, and the four behaviour fixes from this wave under Fixed.

What I checked, and what was already fine

place state
evaluate.py correct since #94
doc/examples/run_env_agent.py correct since #94
doc/examples/test_navigation_agent.py correct since #102
evaluate_scenario_colab.ipynb drives no loop of its own, so nothing to change
"Terminated: Rocket flight finished" correct, that one really is termination

Tests

The two environment fixes are covered in tests/test_episode_lifecycle.py, driving a no-launch episode to the horizon. The frame test also asserts that the final step misses the render cadence, since otherwise the old gate would have drawn it anyway and the assertion would have held against either version.

The README snippet is not a .py file, so the invariant test added in #102 could not see it. It now reads the fenced python blocks out of the README and puts any loop it finds through the same guard and unpack checks as the runners, plus a check that it found one at all, since a snippet that stopped being tagged python would otherwise leave that holding over nothing.

The existing console-logging test pinned the old wording, so it moves too.

Verified by mutation, all five caught:

mutation caught by
the old log wording the lifecycle log test
the old render gate the final-frame test
README shows while not terminated: the README loop test
README discards truncated the README loop test
the snippet stops being tagged python the "found a loop at all" test

Local CI green on a clean tree: ruff check, ruff format, uv lock --check, 315 passed with BPC_RUN_SLOW_TESTS=1.

Two places that still read terminated from when that one flag also
covered the clock. Splitting the two causes was right; neither of these
moved with it.

The log said "Terminated: Reached max time" for the case the flags now
deliberately call truncation. That line is the one thing an operator
reads to find out why a run stopped, so it should not name the wrong
ending.

The renderer drew a frame every 0.1 s of simulation time or when the
episode ended, and the ending it checked was terminated alone. While
that covered the clock, an episode running out of horizon always drew
its last frame. Afterwards it only did so when the final step happened
to land on the cadence. Scenario 0 truncates at step 9999 against a
cadence of 10, so it did not, and scenario 1 ends this way normally.

The existing console test pinned the old wording, so it moves too.

Verified by mutation: both the old wording and the old render gate fail.
The frame test also asserts that the final step misses the cadence,
since otherwise the old gate would have drawn it anyway and the
assertion would hold either way.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 28, 2026 16:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@thc1006 thc1006 mentioned this pull request Jul 28, 2026
Sweeping for everything that still read terminated from when that one
flag also covered the clock turned up two more than the code.

The README described step() as returning "the new observations, reward,
termination flag, and additional info", which is four values and one
flag, and said the episode ends when the time limit is reached or the
rocket lands, as though those were the same outcome. It is the document
a competitor writes their loop from, so it was the worst place for this
to be stale. It now names both flags, says which cause each one is, and
shows the loop.

The changelog had no entry at all. Added under Changed as breaking for
every agent loop, alongside the observation copies, and the four
behaviour fixes from this wave under Fixed.

The README snippet is not a .py file, so the invariant test could not
see it. It reads the fenced python blocks out of the README and puts any
loop it finds through the same guard and unpack checks as the runners,
plus a check that it found one at all, since a snippet that stops being
tagged python would otherwise leave the check holding over nothing.

Verified by mutation: the README showing the old loop, the README
discarding truncated, and the snippet ceasing to be python all fail.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006 thc1006 changed the title Report and draw a truncated episode as one Finish the truncation change: the log, the final frame, the README and the changelog Jul 28, 2026
@thc1006
thc1006 merged commit c0b70c2 into develop Jul 28, 2026
3 checks passed
zuorenchen added a commit that referenced this pull request Aug 27, 2026
* Refuse to pack a submission from a run that never finished

build_submission_payload reads the score and the trajectories straight off the
environment, and nothing said whether the episode had reached an ending. A run
stopped part way through produced a submission that looked exactly like a
complete one, carrying whatever score it had reached by then.

Measured on scenario 0: five steps in, with the rocket still on the pad, the
payload came out fully formed and leaderboard_info had no field that mentioned
the run being unfinished.

The environment now records how the episode ended, reset clears it, the payload
carries it along with the step count, and pack_for_submission refuses a run that
never got there. Which of terminated and truncated should score is #104 and is
not decided here; this only asks whether the episode reached an ending at all.

The fake environments in the submission tests gain the same two fields. They
document themselves as the attributes pack_for_submission reads, and that set
grew.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

* Clean up comments

---------

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Co-authored-by: zuorenchen <zuorenchen@m110.nthu.edu.tw>
Co-authored-by: ZuoRen Chen <180084773+zuorenchen@users.noreply.github.com>
zuorenchen added a commit that referenced this pull request Aug 29, 2026
* Write down that the launch step's control fields are not applied

Closes #80, as decided there: document the behaviour and leave the
implementation alone.

The rocket is still on the rail on the step that launches, so `tvc`, `throttle`
and `roll` would not change where it goes. That step builds the flight from the
launch attitude and the step after it is the first that applies them.

Two lines, one in the README where the action space is described and one in
`step()` where a reader would ask.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

* Say why an action field was ignored

The catch around the conversion is broad on purpose, and the log line it fed
said only that a field was ignored. A competitor sending a string and a defect
in the four lines above it produced the same sentence, so the defect was
invisible.

The reason travels with the field name now:

    Step 42: ignoring tvc, which the environment cannot use:
    ValueError: could not convert string to float: 'x' (1 such steps for that field)

check_action still answers with a sorted list of names, since that is what an
agent calls.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

* Record what landed after v0.1.1

The Unreleased section was empty again within the hour, which is the drift #134
existed to stop. Two entries, for #131 and #126.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

* Put a placeholder where the example carried a real secret

example_eval_cfg.yaml and the README both shipped a working team_secret. A
credential does not belong in an example, and a competitor has to replace it
with their own anyway, so the file now says what to paste instead of shipping
something that already works.

No behaviour changes. The secret is only ever copied into the packed
submission, and nothing reads its value.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

* Fix inclination indication in readme fig (#153)

* ENH: Add wall time limit (#154)

* Add wall time limit

* Update tests

* Update the suggestions from PR review

* Start the episode clock on the same one step reads, and pin it (#156)

* Start the episode clock on the same one step reads

reset() started it on time.time() while step() measured with time.monotonic(),
so the difference was about -1.8e9 and no episode could reach any limit.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

* Pin the wall clock limit with the test that would have caught it

Nothing in tests/ mentioned max_wall_time, and a limit that can never fire
looks exactly like a limit nobody reached.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

---------

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

---------

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Co-authored-by: 秀吉 <84045975+thc1006@users.noreply.github.com>

* Refuse to pack a submission from a run that never finished (#142)

* Refuse to pack a submission from a run that never finished

build_submission_payload reads the score and the trajectories straight off the
environment, and nothing said whether the episode had reached an ending. A run
stopped part way through produced a submission that looked exactly like a
complete one, carrying whatever score it had reached by then.

Measured on scenario 0: five steps in, with the rocket still on the pad, the
payload came out fully formed and leaderboard_info had no field that mentioned
the run being unfinished.

The environment now records how the episode ended, reset clears it, the payload
carries it along with the step count, and pack_for_submission refuses a run that
never got there. Which of terminated and truncated should score is #104 and is
not decided here; this only asks whether the episode reached an ending at all.

The fake environments in the submission tests gain the same two fields. They
document themselves as the attributes pack_for_submission reads, and that set
grew.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

* Clean up comments

---------

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Co-authored-by: zuorenchen <zuorenchen@m110.nthu.edu.tw>
Co-authored-by: ZuoRen Chen <180084773+zuorenchen@users.noreply.github.com>

* Fix pylints

* docs: make README setup commands cross-platform (#138)

* docs: make README setup commands cross-platform

* Potential fix for pull request finding


---------

Co-authored-by: 秀吉 <84045975+thc1006@users.noreply.github.com>
Co-authored-by: ZuoRen Chen <180084773+zuorenchen@users.noreply.github.com>

* Revert "Put a placeholder where the example carried a real secret" (#159)

* Fix: ActiveRocketPy bug fix (#157)

* Update ActiveRocketPy submodule to fix TVC and acclerometer bug

* Fix the incorrect accelerometer model following the updates in ActiveRocketPy

* Regenerate scenario 0 and 1 baselines

* Bump submission version to 2 (#160)

* Update changelog for v0.2.0

---------

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Co-authored-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Co-authored-by: William Mou <william.mou1024@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants