feat(hparams): persist hparams panes on creation and add an update endpoint (Layer 3 - 9) - #1656
Open
Manik-Khajuria-5 wants to merge 55 commits into
Open
feat(hparams): persist hparams panes on creation and add an update endpoint (Layer 3 - 9)#1656Manik-Khajuria-5 wants to merge 55 commits into
Manik-Khajuria-5 wants to merge 55 commits into
Conversation
Add vis.experiment/log_metrics/finish_experiment client methods that POST to a new /experiments/log Tornado handler. The handler records metadata through ExperimentStore over the server's DataStore and mirrors the blob into in-memory env state so a later full-env save preserves it. Covers create/update, metric append+autocreate, and finish (finished/failed). Once an experiment is terminal, further log/metrics writes are rejected: the store raises ExperimentFinishedError and the handler maps it to 409 Conflict, so a finished run's recorded data cannot change after the fact. Validation returns 400 (bad action/params/status), 404 (finish without experiment), 409 (write to terminal). Adds end-to-end + client-shape tests.
Add py/visdom/experiments/query.py: a small, injection-safe query
language ('lr < 0.01 AND acc > 90') that tokenizes and parses into a
predicate AST evaluated as a pure Python walk over a dict (no eval/exec,
no SQL). Supports < <= > >= = != contains, AND/OR with correct
precedence, parentheses, and dtype-aware casting (numeric/string/bool).
build_record() flattens an Experiment into a queryable dict with bare
and namespaced (param./metric./tag.) keys.
Exported from the experiments package. Covered by
py/tests/test_query.py (45 tests: tokenizing, grammar, precedence,
type handling, malformed input, injection-style strings, build_record).
build_record() was annotated `experiment: Any`, justified in its docstring as keeping query.py free of storage dependencies. That rationale was wrong: models.py imports only stdlib and never touches store.py, so importing it costs nothing and cannot cycle. The real cost of `Any` was an invisible, uncheckable contract. Replace it with an ExperimentLike Protocol (structural, so Experiment satisfies it without importing or inheriting anything) plus a _KeyValueLike protocol for Param/Metric/Tag. params/metrics/tags are declared read-only via @Property: as plain attributes they are invariant, which rejects list[Param] against Sequence[_KeyValueLike] and would have failed the concrete Experiment. Collect metrics in one reverse pass instead of a set comprehension plus an O(keys x metrics) latest_metric() re-scan per key. Semantics are unchanged, including latest meaning last logged rather than highest step. Document the two behaviours the code relied on but never stated: bare and namespaced keys are duplicated deliberately (in-memory, per query; the persisted shape is still to_dict()), and bare-name precedence runs built-ins > params > metrics > tags. Tests 45 -> 48: bare-name precedence across a param/metric/tag collision, latest-metric ordering, and a non-Experiment fake proving the decoupling.
Adds the search layer on top of the L2-3 query parser, so the experiments logged by /experiments/log can actually be found again. ExperimentStore.search(query, sort_by, descending) filters experiments through the parser's predicate and sorts them (newest first by default). Records are built once per experiment and reused for both the filter and the sort, and a run missing the sort field always lands last rather than jumping to the front when the sort is reversed. Mixed-type fields sort without raising, since params are user-supplied. POST /experiments/search wires that to HTTP: it validates query/sort_by/ limit/offset/descending, pages the result, and reports the unpaged total so a caller can walk the pages. A malformed query is a 400 with the parser's reason. Queries stay evaluated in Python -- never eval'd, never SQL -- so an injection payload is a parse error, not an execution. Client side, vis.search_experiments(query, limit, offset, sort_by, descending) returns the server's reply; the JSON-decoding plumbing is factored out of _experiment_send so both endpoints share one path. Endpoint documented in openapi.yaml. Tests: py/tests/test_experiment_search.py (42) covering the store filter/sort, the endpoint e2e over a real Application, validation and paging, and the client message shape.
Remove the inline # comments PR-4 added and fold the useful context into docstrings instead, matching the experiments package style. No behaviour change.
Layer 2 continues: after search, ask what actually differs between the runs it found. New visdom/experiments/compare.py holds the diff itself, pure and free of storage the way query.py's build_record is: build_comparison() lines the experiments up and reports, per section (params/metrics/tags), the union of fields, the shared ones every run agrees on, the differing rest, and the per-run values. Metrics diff on their latest observation, the same value a search compares on, so a run found by "acc > 0.9" shows that acc here. A field only some runs carry is a difference rather than a consensus among those that have it. Two comparisons are deliberately not ==. Bools are not numbers, so amp=True and amp=1 differ, matching query.py's refusal to treat a bool as a number. NaN agrees with itself, since a metric NaN in every run is not a difference and calling it one would bury the real ones. ExperimentStore.compare() selects the runs and delegates. Selection is by name or by query, mutually exclusive: both or neither raises rather than guessing which was meant. env_ids compares in the order given, dedupes, and raises KeyError naming any id without an experiment -- a comparison silently missing a run it was asked for reads as a comparison of the rest. A query selects via search() ordered by sort_by/descending and capped by limit; matching nothing is an empty comparison, not an error. ExperimentCompareHandler (POST /experiments/compare) mirrors the search handler's shape and maps those to 400/404. It rejects a bare-string env_ids, which would otherwise iterate into a comparison of runs "r", "u", "n". The three request validators move from ExperimentSearchHandler statics to module level so both handlers share one copy. Client: vis.compare_experiments(env_ids=None, query=None, limit=None, sort_by=None, descending=True). Documented in openapi.yaml (compareExperiments + ExperimentComparisonSection schema). Tests: py/tests/test_experiment_compare.py (50 -- pure diff, store selection, endpoint e2e, validation, client shape); 414 py/tests pass. Verified live against a real server and client: both selection modes, limit, 404/400 paths, injection payload inert with data intact.
Fold the example's trailing # annotations into the prose instead, matching the experiments package style. No behaviour change.
The stubs cover experiment/log_metrics/finish_experiment but stop there, so search_experiments (L2-4) and compare_experiments (L2-5) were the two client methods a type checker could not see. env_ids is spelled Union[List[Text], Tuple[Text, ...]] rather than the obvious Sequence[Text]: a bare str is itself a Sequence[str], and compare_experiments rejects one at runtime, so Sequence would have a checker bless the very call the client raises TypeError on. A tuple is accepted, hence the union rather than a plain List. Both replies are the decoded JSON of their endpoint, named _ExperimentReply for the Mapping the existing experiment stubs return. Verified the stub parameter names and defaults against inspect.signature of the real methods; no type checker is configured in this repo, so nothing else guards the drift.
Compare took either env_ids or a query. The query mode was redundant: search already answers "which runs match?", so compare's copy of it was a second way to do the same thing, reachable only by duplicating search's syntax, sorting and paging into a second endpoint. It also carried a caveat that could mislead. limit truncated the compared set, so shared/differing were computed over only the runs that survived the cap -- correct, but easily read as a diff of everything matching. Compare is now purely "diff these runs": compare(env_ids). To compare a query's matches, search first and pass the ids on, which is one extra call and keeps the diff honest -- it always describes exactly the runs named. The two endpoints now have one job each. Removed with it: the mutual-exclusion checks (nothing to be exclusive with), sort_by/descending/limit on compare, and the QueryParseError path. env_ids is now required, so a missing one is a 400 rather than a fallback to query. The three request validators move back from module level to ExperimentSearchHandler statics, since compare no longer shares them -- web_handlers.py is byte-identical to L2-4 apart from the new handler. Client compare_experiments(env_ids) loses its optional selection knobs; __init__.pyi follows, and _EnvIds drops its Optional now that env_ids is required. Tests: the query-mode cases go; added that search-then-compare composes, that a stale caller still sending query/limit is ignored rather than a 500, and that a traversal env id degrades to 404 rather than a file read (JSONStore._primary_path already guards this) -- the latter replacing the injection test, whose parser surface compare no longer has. 398 py/tests pass. Verified live against a real server and client: compare, search-then-compare, the 400/404 paths, and the traversal id.
shared/differing was all-or-nothing: a field counted as shared only if
every run agreed, so with run-a lr=0.1, run-b lr=0.001, run-c lr=0.1 the
answer was just "lr differs". That run-a and run-c actually match was in
the values map, but the reader had to spot it.
Each section now also carries groups: per field, the runs clustered by the
value they used.
"lr": [{"value": 0.1, "env_ids": ["run-a", "run-c"]},
{"value": 0.001, "env_ids": ["run-b"]}]
shared/differing stay as the at-a-glance "what changed?"; groups answers
the finer "which runs agree?". They cannot disagree: a field is in shared
exactly when its groups are a single cluster holding every compared run,
and shared is now derived from the groups rather than computed twice.
Grouping deliberately does not use a dict keyed by value. Values need not
be hashable (a param may hold a list), hash(True) == hash(1) with True == 1
so a dict would silently merge them and undo the bool rule _same_value
exists to enforce, and NaN never equals itself so it would never group.
_group_values scans with _same_value instead, keeping one definition of
sameness for the module; the run count per comparison is small.
Groups are ordered by first appearance and env_ids within a group keep the
compared order, so the output is deterministic. A run that never logged the
field is in no group for it.
Tests: 7 new in TestBuildComparison (clustering, shared<->groups agreement,
missing field, bool-vs-1, NaN, unhashable lists, ordering) plus a JSON
round-trip through the endpoint. 406 py/tests pass. Verified live: three
runs on two learning rates cluster as intended, including list-valued
params, with shared and groups agreeing on every field.
Reserve /experiments/suggest for the next-run hyper-parameter suggestion
strategy (Optuna-backed), which belongs to a later layer. The endpoint is a
stub: it parses the request like its siblings and replies 501 Not Implemented
with a JSON body ({"status": "not_implemented", "suggestion": null, ...}) so a
caller gets a stable, decodable answer it can tell apart from a real result.
- ExperimentSuggestHandler (web_handlers.py) + route (app.py), mirroring the
log/search/compare handler shape (static wrap_func, @check_auth post).
- Visdom.suggest_experiment(params=None, env=None) client method + .pyi stub;
type-checks params and posts the search space through for the eventual
strategy.
- openapi.yaml: /experiments/suggest documented (operationId suggestExperiment,
501 stub response schema).
- test_experiment_suggest.py: endpoint 501/JSON-body contract and client
message shape.
The Layer-2 experiments client API had no README coverage (L2-1..L2-5 were local-only with docs deferred here). Add an Experiments section — both the API list entry and the per-method Details — covering the full workflow: experiment / log_metrics / finish_experiment / search_experiments / compare_experiments, plus suggest_experiment documented honestly as a reserved 501 stub. Argument names and defaults match the client signatures.
Add py/visdom/data_model/README.md describing the storage abstraction: the DataStore interface (env/layout/undo operations), the JSONStore backend (on-disk layout, in-memory mode, id sanitisation/path-traversal guard, long-id hash fallback, byte-stable JSON, atomic undo writes), how it is wired through Application.storage, and how to add a new backend. Point the env-persistence skill at the layer, which previously referenced only the old serialization path.
Add the client-side entry point for the Layer 3 hyper-parameter view. `vis.hparams()` gathers experiments through the existing `experiments/search` endpoint (same query syntax, optional `env_ids` filter), flattens them into a compact records payload via the new module-level `_flatten_experiments` helper, and creates a dedicated `hparams` window that renders from its content like the properties/embeddings panes. `_flatten_experiments` collapses each run's params/metrics lists into per-run maps, keeping only each metric's latest value, and returns the sorted param/metric key unions so the frontend renders a table/parallel-coordinates view without re-deriving columns. Includes the `.pyi` stub for the new method and pytest covering the flatten helper (unions, latest-metric, heterogeneous/empty/malformed input) and the client message shape (window type, env/win pass-through, env_ids validation and ordering).
Add a `mode` argument to `vis.hparams` ("query" | "env_ids" | "both",
default "both") so the caller explicitly chooses how the shown runs are
selected: by query alone, by an explicit env_ids list alone, or the
intersection of the two. mode="env_ids" requires env_ids and skips the
query; mode="query" ignores env_ids.
Include tags in `_flatten_experiments`: each run now carries a tags map
alongside params/metrics, and the payload exposes a sorted tag_keys union,
matching the server-side build_record which also flattens tags.
Switch the flatten test fixture to TemporaryDirectory with a tearDown so
the seeded JSONStore is removed after each test instead of leaking temp
directories, and extend coverage for the mode paths and tag flattening.
Previously the env_ids selection always went through search_experiments, which reads every environment on disk and returns every experiment over the wire, only for the client to keep the few it named. When no query is in play, route the selection through compare_experiments instead, which reads only the named environments and returns just those runs; a query, when present, still uses search. Also type-check query up front and treat a blank/whitespace query as no query so it takes the by-id path too. Tests stub both read endpoints and assert which one each mode reaches.
Make the three selection modes strict and mutually exclusive in their
arguments instead of leniently combining them:
query -> non-empty query, no env_ids; fetched via search
env_ids -> non-empty env_ids, no query; fetched via compare (reads only
the named environments)
both -> both required and non-empty; search then narrow by env_ids
mode defaults to None and is inferred from which of query/env_ids are
given; an explicit mode enforces its rule and rejects the wrong argument.
A blank or whitespace-only query counts as no query. There is no
"show everything" call: with neither query nor env_ids a ValueError is
raised.
Move the hyper-parameter selection, flattening and window creation off the Visdom.hparams client and into a POST /experiments/hparams handler. The handler resolves the strict query/env_ids/both modes the client used to resolve itself (invalid combinations are 400s), flattens the selected runs via visdom.experiments.flatten_experiments, and writes an hparams window with that content into the env state. For now the handler only writes to state; it does not broadcast the window to connected clients, since the frontend has no dedicated hparams pane to render it yet. The pane is served on the next env load, and broadcasting lands with that pane. vis.hparams is now a thin call: it validates opts and posts query/env_ids/mode/win/opts to the endpoint, returning the created window id, instead of gathering runs and posting to the events endpoint itself. window() learns the hparams type so the content is stored as-is rather than as a plot.
Register an `hparams` pane so `vis.hparams()` windows are drawn in the browser instead of falling through to the plot fallback: - js/panes/HParamsPane.js: functional pane (PlotPane-style React.memo) reading the flattened records payload from window content; renders empty / error / summary states and leaves an .hparams-views seam for the table / parallel-coordinates / SPLOM / filter views of later PRs. - js/settings.js: register the pane in PANES and PANE_SIZE. - py/visdom/static/css/hparams.css + index.html link: pane styling. Switch the /experiments/hparams handler from the persist-only _store_window to the standard register_window, so the pane is broadcast to connected clients and appears live (broadcast was deferred until this pane existed).
Replace the placeholder run list in the hparams pane with HParamsTable, mounted in the .hparams-views seam. Adds: - hparamsUtils: pure sort/format/filter/color helpers; the comparator mirrors the backend order rule (numbers before strings, missing/NaN last in both directions) so the table and server-side search agree. - Column-header sorting plus a tree 'sort by' dropdown with a 3-state direction toggle (asc/desc/off); active sort column is highlighted. - Client-side text filter and per-run selection checkboxes. - 'color by' shades a numeric param or metric column on a ramp off the Visdom blue (#3b5998). - Both dropdowns reuse rc-tree-select to match the environment selector.
- Add a centered, larger pane title: the experiments handler defaults the hparams window title, and Pane tags its bar with a per-type class so the hparams title can be styled without touching other panes. - Draw solid vertical dividers at the param/metric/tag group boundaries, aligned across the header band, header row, and body. - Fix the double horizontal scrollbar by scrolling only the inner table and adding min-width:0 to the flex chain.
…and color-by Adds a Table | Scatter matrix switcher to the hyper-parameter pane and a new HParamsSplom view that renders a Plotly splom trace from the window records. Users pick up to six numeric param/metric axes and an optional color-by metric (same light-to-dark ramp as the table color spine). NaN/missing values are dropped so empty axes never appear. Reuses hparamsUtils for column building and numeric detection; global Plotly and the PlotPane resize pattern; no new deps and no backend/API changes.
…ree views Centralize the numeric-column selection, grouped TreeSelect data, run labelling, and numeric-or-null mapping in hparamsUtils, and move the Plotly-facing snapshot download, notifier, snapshot mode-bar button, and resize observer into a new hparamsPlot module. The table, scatter matrix, and parallel coordinates views now consume these instead of each carrying their own copy.
The top margin was 30px, so Plotly drew the axis titles above the plotting area and the container clipped them, leaving the names half-cut. Widen the margins so titles, the top/bottom range values, and the colorbar all have room, and pin the label, tick, and range font colors for consistent contrast.
… compare extract cellClass into hparamsUtils and consume it from HParamsTable and HParamsCompare, dropping the duplicated inline className concatenation
…updates memoize the parsed pane content so the column, spec, and visible-record chain no longer rebuilds every render; memo the scatter matrix, parallel coordinates, and filter panel; render only the filter row whose entry changed
The pane could compare final numbers but not show how a run got there: every flatten site keeps only each metric's latest value, so the window content carries no history at all. A fifth tab reads it from the server. - new Metrics tab, gated on the table selection like Compare, since falling back to all visible runs would download every run's full history on tab open and draw an unreadable chart - one line per selected run for a metric picked from the union of the keys the selection logged, coloured by position in the unfiltered records so a run keeps its colour as filters and selection change - reads experiments/compare through window.fetch, never jQuery: the document-level ajaxError handler navigates the whole page to error/500, which would destroy the dashboard on a 404 for a deleted run - caches per run, so ticking one more checkbox fetches one run rather than re-reading the whole selection; Refresh re-reads a running run - a series plots against real steps only when every observation has one, otherwise against its own ordinal; logging without a step is the SDK default and mixing the two would place points the data never claimed - repeated steps collapse to one point with the later write winning, so a line ends on the number the table shows; NaN stays a gap in the line - lifts correctPathname out of ApiProvider so the request keeps working under -base_url The view is fetch-on-demand, not live: the pane memoises on window id, so Refresh is the only way to pick up new observations. Plotly's legend toggles a run, and double-click isolates one, but that visibility resets when the metric changes.
Closes out the dashboard: the numbers were only readable inside the pane, and the view tabs claimed the tab role without honouring its keyboard contract. - CSV and JSON export of whatever the pane is currently showing: the selected runs when a selection is active, otherwise the filtered ones, so what lands on disk is what is on screen - CSV headers carry the group (param.lr, metric.acc) because a param and a metric may share a name; missing values and NaN both become empty cells rather than the strings null or NaN, and commas, quotes, newlines and list params are escaped - JSON keeps the window-content shape, so an export can be read back by anything that already reads a hparams window - the tabs now implement the roving tabindex the role implies: arrows move and wrap, Home and End jump to the ends, focus follows selection, and only the active tab is in the tab order - the tab panel is linked to its tab both ways, so a screen reader can say which view it is announcing - Escape closes the filter sidebar and returns focus to the button that opened it, instead of stranding focus in a hidden subtree - a polite live region announces how many runs survive the filters and how many are selected; the table gets an off-screen caption Comments dropped from the metrics files added in the previous commit.
The hparams views grew their own copies of the same things as each view landed. Pull them into one place: - ColumnSelect / ColumnMultiSelect replace four hand-rolled rc-tree-select blocks that differed only in value and tree data. - HParamsMessage replaces eleven wrap-plus-message div pairs. - useHParamsColumns replaces three identical buildColumns memos. - plotRevision builds the datarevision key the three plots each concatenated by hand; the splom now derives its axis style from plotAxisStyle instead of restating it; the pane downloads through hparamsExport's downloadText, which also stops it leaking the object URL. - One sortGlyph for the caret and the direction button. Also give a single-observation run a visible marker: a run with one logged value drew nothing under mode 'lines', so the metrics plot read as empty.
The centred title rule matched every .pull-right in the pane bar, so the comment button was stretched across the bar and its icon landed on top of the title. Scope the rule to the title div and leave the buttons floating.
Contributor
There was a problem hiding this comment.
Sorry @Manik-Khajuria-5, your pull request is larger than the review limit of 150000 diff characters
ExperimentStore read the env it was about to persist from disk, so every metric log rewrote the env file from a snapshot that was missing windows created since the last save and still carried windows the user had closed. The store now takes an env_provider (the server passes state.get) and prefers the live env for both reads and writes; the env may be a LazyEnvData, so it is mutated through the mapping interface (gaining __delitem__ for experiment deletion) and never copied. Logging now persists the environment the server is actually serving, panes included.
The generic /update route only understands plot-shaped content, so an hparams pane could not be changed without re-registering the whole window. POST /experiments/hparams/update rebuilds an existing hparams window in place: a query/env_ids/mode body replaces its selection under the same validation as create, and a bare win re-runs the selection now stored on the window (a manual refresh). The rebuilt window keeps its id and position but mints a fresh contentID, is broadcast to subscribers, and the env is saved so disk reflects the update immediately. Creating a pane also saves the env right away instead of waiting for the next explicit save.
Thin wrapper over the experiments/hparams/update endpoint, next to vis.hparams: pass a selection to replace the pane's one, or only the window id to re-run the stored selection and pick up runs logged since the pane was built. win is required client-side; opts are validated like the other plotting methods and override the pane's title/size when given.
/experiments/hparams was never added to openapi.yaml when the endpoint landed; document it together with the new /experiments/hparams/update, in the style of the other experiment paths. README gains the matching vis.hparams and vis.update_hparams entries next to the other experiment methods, and the type stub gains update_hparams.
Manik-Khajuria-5
force-pushed
the
Layer3-PR10
branch
from
July 28, 2026 09:31
a3b130f to
b47455a
Compare
Manik-Khajuria-5
marked this pull request as ready for review
July 28, 2026 09:59
Contributor
There was a problem hiding this comment.
Sorry @Manik-Khajuria-5, your pull request is larger than the review limit of 150000 diff characters
Manik-Khajuria-5
requested review from
Jayantparashar10,
Saksham-Sirohi,
marcoag,
mariobehling,
norbusan,
rajnisht7,
tonypzy and
vedansh-5
July 28, 2026 10:01
Saksham-Sirohi
requested changes
Jul 28, 2026
Collaborator
There was a problem hiding this comment.
Copilot flagged the following:
- Experiment log handling again has no readonly write guard (same regression as #1602).
search()builds the full experiment list in memory andlimit=Nonereturns unbounded pages.- SPLOM tooltips put
formatValue(...)intoinnerHTMLunescaped — escape that output.
8 tasks
Manik-Khajuria-5
left a comment
Member
Author
There was a problem hiding this comment.
@Saksham-Sirohi I have made a separate PR for addressing these changes PLease refer #1660
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.
Summary
A pane created with
vis.hparams(...)only lived in memory the env was never written, so it was lost if the process died before shutdown. And an existing pane couldn't be changed: the generic/updateroute type-gates on plot-shaped content.Fixes : #1639
Integration note
This PR should be merged after #1647 , as it depends on the lower-layer changes introduced there.
Changes
Envs are written from live state.
ExperimentStoreread the env off disk before writing it back, so/experiments/logpersisted a snapshot that ignoredapp.state. It now takes anenv_provider(handler.state.get) and mutates the live env in place. Logging flushes the real env; closed panes stay closed.Panes are saved on creation via
storage.save_envafterregister_window, asupdate_commentand fork already do.New
POST /experiments/hparams/update.winmust be an existinghparamswindow (404 unknown, 400 wrong type). Aquery/env_ids/modebody replaces the selection; a barewinre-runs the stored one. The window keeps its id andcontentID, and the env is saved.Client:
vis.update_hparams(win, query=None, env_ids=None, mo.Docs: both hparams paths in
openapi.yaml(create was undocumented), READMEsections,
.pyistub.Tests
New
py/tests/test_hparams_update.py, plus cases intest_experimetest_experiment_hparams.pyandtest_hparams.py`. Verified end to end against a running server by inspecting the env JSON on disk.Backwards compatibility
No protocol or signature changes.
ExperimentStore(datastore)with behaves as before. Panes predating this change have no stored selection, so a bare refresh returns 400; passing a selection works normally.