FIX: serialize numpy trial_scores in ScorerMetrics.to_json - #2711
Open
fei (feiiiiii5) wants to merge 1 commit into
Open
fei (feiiiiii5) wants to merge 1 commit into
fei (feiiiiii5) wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #2710.
ScorerMetrics.to_json()documents itself as "the canonical serialization entry point forScorerMetricsand its subclasses", paired withfrom_json_file()for round-trip(de)serialization. It raises
TypeError: Object of type ndarray is not JSON serializableforevery metrics object
ScorerEvaluatorhands back, because the evaluator attachestrial_scoresas a numpy array immediately before returning, with an in-source comment telling callers to use
that returned object for detailed analysis (
scorer_evaluator.py:448-450).Two changes, both inside that documented pair:
to_json()passes adefault=hook that encodes numpy arrays and numpy scalars with.tolist(). It still raisesTypeErrorfor anything that is neither JSON nor numpy, so thefix cannot degrade into silent stringification of a bad value.
from_json_file()decodestrial_scoresback into annp.ndarray, so the round trip returnsthe type the field is declared with instead of a nested list.
np.float64fields (mean_absolute_error,mae_standard_error,accuracy_standard_error) werenever the failing field:
np.float64subclassesfloat, sojson.dumpsalready accepts them anddefault=is not consulted for them — their output is unchanged.ndarray,np.int64andnp.bool_are the numpy typesjsonrefuses (checked against numpy 2.4.4), which is why the hookcovers
np.genericas well asnp.ndarray.Round-trip fidelity, stated precisely:
to_json()writes.tolist(), so the loaded array's dtypeand shape follow the stored lists rather than the original array. Measured on this branch:
to_json()+from_json_file()float642×2 (whatHarmScorerEvaluatorreturns)bool3×2 (whatObjectiveScorerEvaluatorreturns)float321×2float64, values exactnp.zeros((0, 3))(0,)null, loads as object dtypeto_json()fine, decode raisesValueErrorThe last four are not reachable from
ScorerEvaluator, which always assigns a rectangularfloat64orboolarray (np.array(all_model_scores_list)), but they are the honest boundary of"round trip".
from_json_file()decodes only when the stored value is alist, so a hand-writtennon-list
trial_scorespasses through unchanged instead of raising insidenp.array().Deliberately not changed:
_metrics_to_registry_dict()inscorer_metrics_io.pystill excludestrial_scores, so JSONLregistry files keep their current shape;
test_metrics_to_registry_dict_excludes_trial_scoresstill passes untouched.
==between two array-carrying metrics still raisesValueError: The truth value of an array with more than one element is ambiguous, because thedataclass compares the ndarray field elementwise. Whether
trial_scoresshould participate inequality is a semantics decision rather than a bug fix, so I raised it separately in the issue
instead of smuggling
compare=Falsein here. It is also why the new tests assert per fieldrather than reusing the
assert loaded == metricsoracle of the two existing round-trip tests.Tests and Documentation
Three tests added to
TestScorerMetricsSerializationintests/unit/score/test_scorer_metrics.py:test_harm_metrics_to_json_serializes_trial_scores— the reported failure:to_json()onHarmScorerMetricsholding a 2×2float64array, then a file round trip throughfrom_json_file()that comes back as anndarraywith the same shape and values.test_objective_metrics_round_trip_keeps_every_serializable_field— abool-dtype array (whata true/false evaluation produces) plus every other field compared through
dataclasses.asdict()withtrial_scorespopped on both sides.test_to_json_rejects_values_that_are_not_numpy_or_json— pins that the new hook still raisesfor a value that is neither JSON-serializable nor numpy.
The first two fail on the base commit and pass with the patch; the third passes on both by design
(it constrains how the fix is implemented). No documentation example or in-tree caller invokes
metrics.to_json()today, which is why the existing suite stayed green — so there are nodoc/changes and no JupyText run for this diff.
Verification
Environment for everything below: macOS 27 arm64, Python 3.14.5, numpy 2.4.4, dependencies
resolved by
uv sync --frozen --extra allfrom this branch's ownuv.lock(the same dependencyset CI resolves). Every command was run with
cwdinside the PR worktree unless stated.Old behavior vs new, same test file both sides (
-p no:randomlyon both so the runs arecomparable):
5172897(detached worktree, this PR's test file copied in, source untouched)pytest tests/unit/score/test_scorer_metrics.pyTypeError: Object of type ndarray is not JSON serializable / when serializing dict item 'trial_scores'intest_harm_metrics_to_json_serializes_trial_scoresandtest_objective_metrics_round_trip_keeps_every_serializable_fieldtest_to_json_rejects_values_that_are_not_numpy_or_jsonpasses on both sides on purpose; it is aguard on how the fix is implemented, not evidence about the bug.
Project checks on the head commit:
uv run --frozen --extra all python -m pytest -n 4 --dist=loadfile --cov=pyrit --cov-fail-under=78 tests/unit --cov-report xml --cov-report term -q→18181 passed, 11 skipped,Required test coverage of 78% reached. Total coverage: 95.07%uv run --frozen --extra all python -m diff_cover.diff_cover_tool coverage.xml --compare-branch=origin/main --diff-range-notation=.. --fail-under=90→pyrit/score/scorer_evaluation/scorer_metrics.py (100%),Total: 10 lines Missing: 0 lines Coverage: 100%uv run --frozen --extra all pre-commit run --files pyrit/score/scorer_evaluation/scorer_metrics.py tests/unit/score/test_scorer_metrics.py→ all hooks Passed, includingruff-format,ruff-check,ty (type check),Reject Sphinx reST cross-reference rolesandEnforce _async Suffix on async defuv run --frozen --extra all python -m pytest tests/unit/score tests/unit/cli/test_import_guards.py -q→1980 passed. The import-guard tests are included because they are what would object toimport numpymoving out of theif TYPE_CHECKING:block;scorer_evaluator.pyandkrippendorff.pyalready import numpy at module level, so this matches the package.Reachability check outside the unit tests (a one-off script, deliberately not added to the suite):
driving the real
HarmScorerEvaluator.evaluate_dataset_async(...)with the mocked-scorer recipe oftests/unit/score/test_scorer_evaluator.py:75— on base the returned metrics raise theTypeErrorfromto_json(); on this branch the same call returns... "trial_scores": [[0.2, 0.4], [0.2, 0.4]], .... The round-trip table under Description comesfrom the same script, run against this branch.
Not verified here: CI on Windows/macOS runners and the notebook-based docs examples. No
doc/file is touched, and per the description above no in-tree caller or docs example reaches
metrics.to_json(), so JupyText was not re-run. I will report what the workflow checks show oncethey are permitted to run for this fork.