Skip to content

Say why an action field was ignored - #131

Merged
thc1006 merged 1 commit into
developfrom
fix/say-why-an-action-field-is-unusable
Jul 31, 2026
Merged

thc1006 merged 1 commit into
developfrom
fix/say-why-an-action-field-is-unusable

Conversation

@thc1006

@thc1006 thc1006 commented Jul 30, 2026

Copy link
Copy Markdown
Member

From a review of #117: the catch around the action conversion is except Exception, so a defect in our own code, or a third-party exception, is reported
as ordinary invalid input.

Three parts to that, and they are not equally true.

The catch should stay broad. It is there because the list cannot be guessed.
A torch tensor still carrying its graph raises RuntimeError on np.asarray, an
action that is not a mapping raises TypeError on the lookup, and every narrowing
so far has been one of those found the hard way. except Exception also does not
catch KeyboardInterrupt or SystemExit, which are BaseException.

But the message was the problem, and this fixes it. The log said:

Step 42: ignoring tvc, which the environment cannot use (1 such steps for that field)

Nothing about why. A competitor sending "forward" and a NameError in the four
lines above produced that same sentence, so the defect was invisible. Now:

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

_usable_action_fields returns a mapping of field to reason instead of a set of
names. check_action still answers with a sorted list of names, because that is
the public thing an agent calls:

if check_action(action):
    action = last_good_action

The __array__ part does not apply here. np.asarray does call __array__ on
whatever it is handed, so an expensive or side-effecting one runs. That object comes
from the competitor's own agent, in the competitor's own process, and get_action
can already run anything. Nothing here re-executes a submitted agent: I checked, and
agent_module_file is carried in the submission but never imported or executed. If
that ever changes, the sandbox question is much bigger than this function.

Tests

Eight, in tests/test_unusable_action_reasons.py. Drop the reasons and go back to a
set: six fail, and the two check_action tests pass, which is the point of having
them.

a raised exception keeps its type fails
an unexpected exception type survives fails
the wrong count says what was expected fails
a non-finite value says so fails
a missing field keeps its lookup error fails
the reason reaches the log, through env.step fails
check_action is a sorted list of names passes
a usable action is an empty list passes

Local CI is green: ruff check, ruff format, uv lock --check, 506 passed and 22
skipped.

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>
@thc1006

thc1006 commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

@zuorenchen this sits on the same ground as #117 which you said went a bit too deep, so please close it if you would rather not carry it; all it does is make the existing log line name the conversion that failed, so a competitor can see what to fix in their own agent.

@zuorenchen zuorenchen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, let's merge it as it is working nicely

@thc1006
thc1006 merged commit 21a5029 into develop Jul 31, 2026
5 checks passed
@thc1006
thc1006 deleted the fix/say-why-an-action-field-is-unusable branch July 31, 2026 11:51
zebra314 pushed a commit to NYCU-iTron/BalloonPoppingChallenge-2026 that referenced this pull request Aug 7, 2026
`says which field and why` describes ARRC-Rocket#131, which is still open. Without it the
log names the field it dropped and not the reason, so the line said more than
the release does.

Signed-off-by: thc1006 <84045975+thc1006@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