Standalone OG-CORE Ingestion Pipeline Backend - #498
Conversation
|
Hey @error9098x , @utsinboots Would appreciate if you could test the same aswell ! |
|
Tested
All the above tests 1: Cancelling a queued run leaves it permanently stuck as It never transitions to 2: Let me know if I missed anything and I'll give it another go. Also, curious to know, how do I test the automated test suites you described in the PR? |
Cancelling a queued run only dropped it from the queue, so it stayed pending forever: nothing revisits a pending run, only a running one is repaired on a status read. It now gets the same terminal state a cancelled active run gets. Result tables named the base slot "Baseline" whichever run was in it, so viewing a reform on its own called it a baseline. OG-Core labels the two slots by position and takes no label argument, so a one-run table is relabelled with the run name on our side. Comparisons still read baseline/reform. Applies to the macro, inequality, gini and time series tables, and to the CSV download. Also stops an unwritable run_meta from breaking the queue drain and stranding everything behind it, and adds pytest coverage for the queue, cancel, labelling, run guards and restart recovery.
|
Hey @utsinboots , thanks for catching those , fixed them in the latest commit. Certain changes pertaining to full correctness with the 503 fixes remain and are not folded in currently. Ill work on those once 503 is in main. Lemme know if you find anything else that needs to be fixed :) |
|
Tested the latest changes |
The parameter form came back empty for every country calibration: 129 of 133 parameters had no title, description, default or range. A country defaults file lists plain values while the base file carries the metadata, and the overlay projected both the same way, so it replaced everything it touched. The overlay now keeps the base metadata and swaps in the country's own value, which is also the value the user actually has. Deleting a baseline removes the whole case, but unlike deleteCase it asked for no session, so it was an unguarded case delete. It now clears the same gate; deleting a reform is unchanged. Parameters could also be changed while a run was already running or queued. The worker reads them when it launches, so that either left a finished run's saved parameters disagreeing with its results, or let a queued run slip past the dimension check it had already passed. saveParams and uploadTaxParams now refuse until the run finishes. Also: reforms record their baseline by name and resolve the path at launch, so a case restored on another machine still finds it; run_meta is written atomically since the status endpoints poll it, and one unreadable meta no longer empties the run list; restoreCase caps the upload and what it may expand into; run ids are no longer reused after a delete; and the reform dimension check now includes consumption goods alongside S, T, J and M.
# Conflicts: # API/app.py
|
@error9098x , implemented some of the changes you recommended , thanks for those. On the validation of Params - it currently retains its original state of atomic validation of bounds, type etc. I tried out the approach you had mentioned to check conflicting attributes and validate them , but the recommended method did not work out. We can possibly enforce hard correlation rules but that would be a stretch, decided against doing that. |
Installing or updating a calibration rewrites the venv a solve is running under, so the two now refuse to overlap: a run will not start while an install is in flight for that country, and installing, updating or re-registering a calibration is refused while a run is using it. A first install is unaffected, since a country with nothing installed cannot have a run. The worker was spawned without -u. Writing to a pipe, Python block-buffers stdout, so a solve's progress sat in the child until it exited: an hour-long run showed one log line and never reported an iteration. It is spawned unbuffered now. A run left behind by a crash is repaired at startup as well as on the next status read, and its worker is killed if it outlived the server. The kill only fires when the recorded pid is still that run's worker, so a reused pid is never touched, and a run directory no longer matches one it is a prefix of. Stopping the server stops a running solve alongside a running install. Also: getRunStatus reports why a run failed rather than only that it did; a failed status read no longer overwrites a run that finished in the meantime; cancel kills outside the lock so it cannot block status polls; the wealth moments table accepts the data moments OG-Core needs to build it at all; and runs get an optional inactivity ceiling, off by default because a healthy solve can be quiet for a long time.
A malformed MUIOGO_OGC_RUN_TIMEOUT_SECONDS raised at import and took the whole app down, while the inactivity value beside it fell back quietly. Both read through one helper now, so a typo in a tuning knob leaves the default in place. Also covers the repair that only fires after a re-read, so a solve that finishes while its status is being polled is not written off as failed.
Without -ww, ps clips the command to the terminal width, or to $COLUMNS when there is no terminal. The run directory sits at the end of a worker's command line, so it was being cut off and the orphan check could never match: on Linux and macOS a leftover worker would simply never be recognised. It surfaced as a test failure because pytest sets COLUMNS.
Thanks for explaining that. That makes sense, and I don't see this as a blocker for the PR. I mainly wanted to confirm the behavior because these invalid shapes can pass the atomic validation and fail later inside OG-Core with a low-level error. Since you are keeping validation focused on individual bounds, types, and formats, I will treat the cross-parameter checks as a known limitation rather than a required change here. |
OG-Core builds this table from a "Data" column of survey moments, and leaves that column empty when none are given, so the frame cannot be built at all and the endpoint failed for every calibration. Hand it blanks and drop the column afterwards, which leaves the model's own numbers. A scalar broadcasts, so this does not depend on how many moments the table has. Passing real data moments still returns both columns.
OGResults.run_completed and OGRunner.alive were written during the build and never called from anywhere, including the tests. The run layer's own docstring still described a single wall-clock watchdog, from before the inactivity ceiling and the orphan kill were added. The rest is whitespace left by earlier edits, and one copy of a test helper that existed in two files. No behaviour changes.
Functional testing on a deployed backend turned up two small gaps in the analysis tables. time_series took no options through the API while the worker behind it accepts stationarized, so the option could not be reached from outside. Same route/worker mismatch as the macro table output_type and the wealth moments data_moments. Asking for the macro table with only a steady state solved was refused with the path of the pickle it could not find, which tells the caller nothing they can act on. It now says there are no transition path results and to run with the full time path, matching what getResults already said. Only the transition path case changed; a missing steady state or model_params still names the file, since neither is something a caller can fix and the path helps when diagnosing one.
|
Conducted extensive functional tests by deployment on an active server with only API based I/O
The tests that were covered:Server health
Installing calibrations
Cases
Runs and parameters
Running a solve
The queue
Cancelling
Interruption and recovery
Runs and installs together
Results and analysis tables
Export, backup and restore
Refusals and bad input
The team can use these as templates for functional testing, feel free to extend this list if ive missed anything important, we can cover the same to check every bit of the Frontend-Backend bridging and functionality. Another update is that the current PRs pertaining to the frontend made by Aviral are already based on the Backend Infra in this PR, functionality is being tested while frontend is being developed as well, hence solidifying it further. |
|
@marcelolafleur Ready for review! |
There was a problem hiding this comment.
Thanks @Adityakushwaha2006, this is careful work. I read all of it: the process boundary is clean (MUIOGO itself never touches ogcore), runs recover properly after a crash or restart, and the tests exercise the real queue and shutdown paths. Approving.
One fix before merge: restoreCase unpacks the backup straight into the final case folder. If extraction fails halfway (disk full, or a file name Windows refuses) the user is left with a broken half-case, and restoring again is blocked because the case now "exists". Unpack to a temporary folder and move it into place only once everything succeeded.
restoreCase unpacked the backup straight into the final case folder. A restore that stopped partway, on a full disk or a name the filesystem refuses, left the files written so far sitting under the case name. That wreckage read as a real case, and it also blocked the obvious fix of simply trying again, because the name now existed. Unpack into a staging directory and publish with one rename, so a failed restore leaves nothing at all. Staging goes beside the cases directory rather than inside it: same filesystem, so publishing stays a rename, and a restore in flight is never visible to the two things that walk the cases directory, list_cases and the startup reconcile pass, both of which treat any directory there as a case. The move can still lose a race, since the earlier existence check is not a lock, so a failure there reports the case as already existing when it now does. Nothing is published in that case either. Tests cover the round trip, the refusal to overwrite, and the three things the old code got wrong: no half-case is published, the retry after a failure works, and nothing appears under cases while unpacking.
|
@marcelolafleur All done now :) |
|
@Adityakushwaha2006, is it possible to store cases using a country-specific directory structure? Currently, cases are stored as: This can cause name collisions when different country workspaces use the same case name. The country-aware structure may need to be reflected in storage, API lookups, queue handling, workers, and results handling. |
Case names were global, so "Baseline" could only exist once across every country. Creating it a second time either hit a confusing refusal about country_id or, if the client left country_id out, silently edited the other country's case. Cases now live at cases/<country_id>/<casename>. A case is identified by the pair, so country_id is required wherever an endpoint names one, and the session carries both halves. The directory a case sits in decides its country: genData is pinned to it on write, and the listing reads it from the path. Two things fall out of that. is_country_running answers from the run's own key instead of reading genData, so an unreadable case cannot hide a live run from the install guard. And an edit can no longer move a case between countries, which retires the country_id immutability check. Cases stored in the old layout are moved under their country at startup. One that records no country is left alone rather than guessed at.
|
@error9098x Ive implemented the requested change , I could see the problem you were running into , tried to make a way around it but then decided to implement the recommendation itself. A note for you , and others , this change breaks away from the API contract we had laid out , and also the access point-save point operations in some cases internally, that might already be in the frontend's code that has been merged. Ive ran my functional tests and my pytests on this to verify, need to note that every endpoint in the affected tree would ask for country id aswell. The diff for the particular commit would help pin point exactly where the API follow-through changes internally , and that'd be the first thing i'd recommend changing in the frontend. Let me know if this works fine , and if you need anything else. |
Summary
What changed: adds the OG-Core run pipeline, the part that runs an installed calibration and serves its results , fully end to end. Thirty ogc endpoints(as per specific contract) for cases, runs, parameters, execution, results, the analysis tables, tax uploads, the parameter form schema, and case backup and restore. All the model work is done by a permanent worker script that runs OG-Core in its own country specific environment; MUIOGO never imports ogcore for in process runs (issue with #471), All calls are sub processes.
Why: This is the layer that runs the entire computation for OG-CORE : the frontend can now create a case, solve a baseline and a reform, and read the results back. It is the cross-environment run layer that was mentioned under #470.
Architectural Explanation
flowchart TB FE["Frontend (browser)"] -->|"HTTP /ogc/..."| RT["OGCoreRunRoute<br/>web layer"] RT --> JOB["RunJob<br/>one solve at a time, queue, cancel"] RT --> ST["OGCoreCase<br/>case and run files on disk"] JOB --> RUN["OGRunner<br/>spawns and watches the worker"] RUN -->|"looks up python_path"| REG["Installed registry (from #486)"] RUN -->|"spawns with the calibration's own python"| W["ogc_worker.py<br/>the only code that imports ogcore"] W --> OG["OG-Core, in its own environment<br/>solve, then read its own pickles,<br/>write plain JSON results"] RT -->|"results endpoints read plain JSON"| DISK["run folder on disk"] W --> DISKHow a Run happens :
Additional Things to note:
getRuns/getRunStatusand the frontend just resumes polling; if the whole server was restarted mid-run,getRunStatusrepairs it to failed ("interrupted by restart"). Nothing is lost either way.To Obtain the entire case-run tree on the frontend:
Validation
Tests added/updated
Automated suites across the build: driving 12 real model solves through the full stack (steady state and transition path, baseline and reform, plus the queue, a cancel, and a retry after cancel). Every guard, every error shape, and every result endpoint was exercised. Full Parity achieved with direct OGC runs on sample data - proving that there is prevalent logical/computational bug in the processing pipeline. All tests have been conducted on the base model.
Manual verification steps documented, with evidence where relevant (attached below)
Live Manual Verification :
Live session against the running backend. Every call is a real HTTP request.
Reference : installed base metadata in installation registry json
1. A real model solve, completed through the API
Shows a run started through
/ogc/run, polled through tocompleted, with the livestage and iteration count climbing as it solved. This is a full transition-path solve
of OG-Core, launched by the pipeline.
2. The parameter form schema
getParameterSchemareturning all 129 parameters, each with its label, grouping,default, and range, read live from the installed calibration.
(Verified everything is returned )
3. Environment available, execution slot free then locked
Two terminals side by side. Before the run, the installed environment is listed and
the run is pending (the slot is free). During the run, the same environment is still
listed and the run is now running (the slot is occupied). The environment path is
always available.
4. Two runs: one running, one queued
A second run scheduled while the first is still solving comes back as queued, and its
status shows pending with a Queued stage. This is the one-solve-at-a-time rule in
action.
5. Cancel, and the queue advancing on its own
Cancelling the running solve returns cancelled, the run then shows failed with the
reason "Cancelled by user.", and the queued run moves to running by itself. This shows
cancel works and the queue advances automatically.
6. Consolidated results (baseline vs reform)
getResultsreturning the dashboard object: the years, each variable's baseline path,its reform path, and the percent difference, plus the steady-state values.
7. Analysis tables and the CSV download
The analysis tables (macro, inequality, gini, time series, revenue decomposition)
returning real rows, computed live from the saved results, and the macro table
downloaded as a CSV file.
8. A prior run fetched from a fresh terminal
A run that finished earlier, fetched from a brand-new terminal with no shared session:
its status, its saved parameters, and its results all come back. This shows run
results are saved durably and are readable later, independent of the session that
created them.
Files
New:
API/Classes/OGCore/OGCoreCase.py(cases and runs on disk)API/Classes/OGCore/ogc_worker.py(the OG-side worker, the only code that importsogcore)
API/Classes/OGCore/OGRunner.py(spawns and supervises the worker)API/Classes/OGCore/RunJob.py(one solve at a time, queue, cancel)API/Classes/OGCore/OGResults.py(reads the worker's plain JSON into the dashboardshape)
API/Classes/OGCore/OGTables.py(drives the worker's short table and check calls)API/Classes/OGCore/OGSchema.py(builds the parameter form metadata)API/Routes/OGCore/OGCoreRunRoute.py(all the endpoints above)Changed:
API/Classes/Base/Config.py(the cases storage path)API/app.py(registers the new blueprint)Checklist
main; PR targetsEAPD-DRB/MUIOGO:mainAdditional Info:
flowchart TB subgraph BROWSER["Browser (frontend)"] FE["JavaScript<br/>sends HTTP requests<br/>renders results"] end subgraph FLASK["MUIOGO Flask app, its own environment, port 5002"] subgraph ROUTE["OGCoreRunRoute.py<br/>WEB LAYER"] R1["receives HTTP request"] R2["validates structure<br/>fields present, names safe,<br/>cross-site guard"] R3["passes raw values<br/>as function args"] R4["wraps return dict<br/>in jsonify"] end subgraph JOB["RunJob.py<br/>JOB LAYER"] J1["runs the two guards"] J2["claims the run,<br/>queues if one is solving"] J3["tracks status,<br/>cancel, timeout"] end subgraph RUNNER["OGRunner.py<br/>LAUNCH LAYER"] L1["looks up python_path<br/>in the registry"] L2["spawns the worker<br/>with a clean env"] L3["streams output to log,<br/>extracts iteration count"] L4["kills the process tree<br/>on cancel or timeout"] end subgraph STORAGE["OGCoreCase.py<br/>STORAGE LAYER"] S1["case CRUD, genData.json"] S2["run CRUD, run_meta.json"] S3["params read/write,<br/>ogcParams.json"] S4["name safety guard"] end end subgraph REG["Installed registry (#486)"] RG1["country_id to<br/>python_path"] end subgraph DISK["Disk: DataStorage/OGCore/casename/"] D1["genData.json"] D2["res/runname/run_meta.json"] D3["res/runname/ogcParams.json"] D4["res/runname/run_status.json"] D5["res/runname/SS + TPI pickles"] D6["res/runname/results_ss.json<br/>results_tpi.json"] end subgraph OGENV["OG environment: the calibration's own .venv, SEPARATE PROCESS"] W["ogc_worker.py<br/>the only code that imports ogcore"] OG1["Specifications<br/>defaults + layers + overrides"] OG2["runner: SS solve then TPI solve"] OG3["output_tables, validator"] end FE -->|"HTTP POST/GET"| R1 R1 --> R2 --> R3 R3 -->|"run endpoints"| JOB R3 -->|"CRUD endpoints"| STORAGE JOB --> RUNNER RUNNER -->|"python_path"| REG RUNNER -->|"spawn subprocess"| W W --> OG1 --> OG2 W -->|"reads own pickles,<br/>writes plain JSON"| D6 OG2 -->|"writes pickles"| D5 W -->|"stage updates,<br/>terminal status"| D4 STORAGE -->|"file I/O"| DISK R3 -->|"results endpoints<br/>read plain JSON only"| D6 JOB --> R4 R4 -->|"HTTP response JSON"| FE