Add PIR (Physics Intermediate Representation) symbolic regression method - #210
Conversation
Added metadata for the Physics Intermediate Representation (PIR) project, including authors, paper title, and description.
Implement PIRClassicRegressor and model function for SRBench.
Added metadata for the Physics Intermediate Representation (PIR) including authors, paper title, and description.
This file implements a classical PIR regressor for SRBench, including configuration and model handling.
|
12/44 EXACT blind (zero seed wobble), v3.4 |
|
Update: The install failure is resolved. The physics-engine package had a packaging issue (subpackages weren't being discovered by setuptools) plus some eager top-level imports pulling in optional modules that aren't needed for the regressor. I've fixed the packaging config and made the top-level imports lazy, tagged v3.4.1, and updated install.sh to point at it. A clean-room install (pip install "git+https://github.com/Qazi-pk/[email protected]" in a fresh venv) now imports PIRRegressor successfully. The remaining ModuleNotFoundError: No module named 'methods.pir' looks like the shared algorithms/ → methods/ harness mapping that's also failing other methods in this run (brush, eql, bsr, e2et). Happy to adjust the layout if there's a preferred structure — otherwise it appears to be on the infrastructure side. Thanks for taking the time to review. |
… ModuleNotFoundError: methods.pir)
|
Update: the Cause: my submission only had the regressor under Fix:
Thanks for your patience with the back and forth. |
|
thanks for your patience. we finally fixed the other methods that were failing in docker on main. once the tests pass we'll perform our review and update you. |
|
Thanks for the update, and for fixing the Docker failures on main. While benchmarking PIR locally through the ground-truth harness, I found and reported a separate bug — #216: -target_noise and -feature_noise are parsed but never passed to evaluate_model(), so target noise is silently ignored on --local runs (the cluster path is unaffected). It includes a two-line fix that worked for me. Happy to open a PR for it if useful — flagging here since it affects local noise-robustness runs. Looking forward to the review. |
every push and PR rebuilds all 27 images, because the gate in build-and-test is hardcoded to should_run=true (5b13029). check-changes already computes what changed, but nothing consumes it. one method submission can cost a lot: #209 went through 16 full 27-image runs, #210 and #212 another 24 between them. pull requests now build only the methods they touch. everything else - pushes to master/dev, the new weekly schedule, manual dispatch - still rebuilds everything, so a method that breaks from upstream drift without anyone touching it still gets caught. that drift is calendar-driven, which is what the schedule is for; during a quiet stretch there are no merges to catch it. - a method rebuilds if either algorithms/<name>/ or experiment/methods/<name>/ changed. the second one matters: a regressor.py edit has to retest the method even though the install is untouched. - changes to shared build inputs (dockerfiles, base_environment, scripts, entry.sh, configure.sh, workflows) still rebuild everything. - build-and-test always runs and always reports for every algorithm, so the check names stay present and can be marked required. only the docker build step is skipped. - dropped always() from build-and-test. with the gate inside the job, a failed check-changes would have left an empty build list, skipped every build and reported green. - check-changes no longer diffs against github.event.before, so a force-push to a CI branch no longer fails the job. also fixes a long-standing bug: changed-experiments used awk field $2 on experiment/methods/<name>/..., which is the literal string "methods", not the method name. it needs $3. nothing consumed that output before, so it never showed up.
scripts/check_method_layout.py (added on master after this PR's branch diverged) requires algorithms/<name>/metadata.yml to have top-level name, authors, email, description, and url keys unless the method is grandfathered. pir isn't in the grandfathered set, and the existing email/url were only nested under authors[]/paper, not top-level - causing the validate-layout check to fail.
|
Hi @Qazi-pk, Thanks for your PR and for your patience. I appreciate that you managed to fix all the CI issues and that the algorithm image now builds smoothly. Also, thanks for making There are a few potential issues that need to be fixed before merging. 1. Complexity doesn't count some operators — since the split removes operators, it only counts operand-like tokens. SRBench will normally use its SymPy complexity first, so this fallback is only used when that fails — but it's still worth making it as accurate as possible. # Count operands and operators as a proxy for expression tree size.
tokens = re.split(r"[\s\(\),\+\-\*\/\^]+", expr)
return len([t for t in tokens if t])One solution would be to also count the operators you're using as the split criteria and add that count to the total. 2. Variable name matching can silently fail. cols = list(X.columns)
# If any real column name already appears, assume names are correct.
if any(str(c) in expr for c in cols):
return exprIf a feature name is similar to an operator, or is a substring of one, this can fail — for example, if you have a feature called 3. Your use_alarm = (
hasattr(signal, "SIGALRM") and self.max_time and self.max_time > 0
)
old_handler = None
if use_alarm:
old_handler = signal.signal(signal.SIGALRM, _on_alarm)
signal.alarm(int(self.max_time))
try:You could remove the |
…unt, word-boundary column matching, remove process-global SIGALRM (relies on SRBench's outer timeout); remove stale duplicate algorithms/PIR/regressor.py
|
Thanks for the thorough review, and for the specific fixes — all three make sense.
Also removed a stale duplicate regressor.py under algorithms/PIR/ that Pushed as 969e323. |
…itivity duplicate of the correct lowercase algorithms/pir/, orphaned since the July regressor.py cleanup. Fixes check_method_layout.py failures.
|
Thank you @Qazi-pk — I was just about to comment about the uppercase Merging! |
PIR — Physics Intermediate Representation
Classical symbolic regression engine for automated physical law discovery from data.
Method
Blind Feynman Tier A results
12/44 EXACT blind (zero seed wobble), v3.4
Secondary: +12/44 FORM_NUMERIC (correct functional form, transcendental constant as decimal — reported separately, never summed)
Previous baseline: 7/44 (v3.3)
Files
algorithms/PIR/regressor.py— sklearn-compatible wrapperalgorithms/PIR/install.sh— installs from public repo (MIT license)algorithms/PIR/metadata.yml— method metadataalgorithms/PIR/requirements.txt— POT (Python Optimal Transport)Links
Targeting master per @gAldeia's guidance in #203.