Update paper to current Adaptive and fix errors#1
Open
basnijholt wants to merge 4 commits into
Open
Conversation
akhmerov
reviewed
Jun 10, 2026
| while queue.max_priority() < target_loss: | ||
| loss, subdomain = queue.pop() | ||
| while queue.max_priority() > target_loss: | ||
| _, subdomain = queue.pop() |
There was a problem hiding this comment.
This change is ok in source, but not ok in the manuscript
Member
Author
There was a problem hiding this comment.
I actually just changed it to max_loss
- Fill in author list (replacing the 'Tinkerer' placeholder) and draft acknowledgements and author-contribution statements - Add missing MCMC/Bayesian OED references (Chaloner & Verdinelli 1995, Ryan et al. 2015) and fix the inverted claim about closed-form solutions - Fix bugs in the pseudocode: inverted while condition, shadowed 'loss' variable, wrong loop variable in queue.remove, missing colon, and a wrong variable in the initial queue.insert of the parallel version - Fix broken Python in the API examples: indentation, misplaced bracket in the BalancingLearner list comprehension, undefined names - Update examples to the current Adaptive API: Runner(loss_goal=...), ask() returning loss improvements, peak() naming - Update 'Possible extensions': anisotropic meshing is now implemented in LearnerND (anisotropic=True) and AverageLearner1D exists; mention the optional Rust-accelerated triangulation backend - Mention concurrent.futures and loky among supported executors - Fix isoline/isosurface dimension mix-up and various grammar issues - Only emit \email in the revtex template when an author has one
The examples in the paper now use the current API (loss_goal etc.), so pin the submodule to a version that actually has it instead of v0.8.1. The figure notebooks only use APIs that still exist (runner.simple, curvature_loss_function, Learner2D.ip, LearnerND._get_iso, utils.load). Also rephrase the anisotropic-meshing and Rust-triangulation mentions: we have no benchmarks for either in this paper, so describe them as implemented/aimed-at rather than claiming improvement.
basnijholt
force-pushed
the
update-paper-to-current-adaptive
branch
from
f20ba33 to
dc710f9
Compare
Counting line segments says nothing about how close the estimated isoline is to the true one. Compute the mean and Hausdorff distance to the exact isoline y = cbrt(level - x^2) instead, in the notebook and in the figure caption: adaptive sampling is a factor 6 (mean) and 2 (Hausdorff) more accurate than homogeneous sampling at equal point count. Numbers computed with the adaptive version pinned in adaptive_repo; the committed isoline.pdf still needs regenerating in the Docker environment (homogeneous triangulation differs slightly: 45 vs 43 segments).
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
2 participants
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.
Brings the draft up to date with the current state of adaptive and fixes a bunch of errors in the text and code listings.
Content updates
adaptive_reposubmodule from v0.8.1 to current main, so the code examples in the paper match the pinned version. The figure notebooks only use APIs that still exist (runner.simple,curvature_loss_function,Learner2D.ip,LearnerND._get_iso,utils.load).LearnerNDfeature (anisotropic=True, Add anisotropic meshing to LearnerND adaptive#141) andAverageLearner1Dfor stochastic 1D functions. Both are described as implemented but not yet benchmarked — no performance claims without data.LearnerND.Runner(learner, loss_goal=0.01)instead of a hand-written goal function,ask()returning loss improvements, andconcurrent.futures/lokyadded to the executor list.Fixes
whilecondition was inverted, the poppedlossvariable shadowed theloss()function (nowmax_loss),queue.remove(old_subdomain)used the wrong variable, aforwas missing its colon, and the parallel version's initialqueue.insertreferenced an undefined variable.BalancingLearnerlist comprehension,adaptive.LearnerNDvs. the importedLearnerND.Front matter
papercodebib entry lists three paper authors (Bas, Joseph, Anton) while the Zenodo record lists four (incl. Jorn), so the author list needs a decision; the acknowledgements don't mention funding yet; and Jorn's affiliation is unverified.\emailfor authors that have one.