Remove dead code, fix correctness bugs, implement sign_rev#59
Open
dpsanders wants to merge 2 commits into
Open
Remove dead code, fix correctness bugs, implement sign_rev#59dpsanders wants to merge 2 commits into
dpsanders wants to merge 2 commits into
Conversation
Delete unreferenced files and helpers: - src/decorated.jl (not included; references DecoratedInterval, which no longer exists in IntervalArithmetic v1) - src/pave.jl (pre-Julia-1.0 syntax; not included) - src/powers.jl (square!/cube!/constant_contractor; unused) - examples/examples.jl (references a non-existent square function) - test/Non1788tests/script.jl (scratch file not in runner) - exp!/log!/asin! mutating helpers (unused by any *_rev function) - Dead #= ... =# body of sign_rev and its export Fix correctness bugs: - asin_rev now intersects x_new with the input x (was ignoring it, so x_new could exceed a tighter input bound) - max_rev/min_rev return all-empty when a, b, or c is empty; guard interval(lo, hi) construction against lo > hi (removes ~9 of the 10 ill-formed-interval warnings during the test run) Docstring cleanup: - "byt default" -> "by default" (10 sites) - cosh_rev / tanh_rev docstrings said "Reverse square root" - Add default x = entireinterval(b/a) to pow_rev1/pow_rev2, matching their docstrings Test suite unchanged (1344 pass, 241 broken). Two Non1788 tests that asserted the buggy min_rev/max_rev empty-a behavior were updated to expect emptyinterval(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous sign_rev body was commented out with broken syntax while
still being exported. Replace it with a working implementation:
sign(x) ∈ {-1, 0, 1} for real x, so the preimage of an interval `a`
under sign is determined by which of those three values lie in `a`.
Include 1 → positive half-line, include 0 → {0}, include -1 → negative
half-line. The tight outer enclosures are [0, ∞), [0, 0], (-∞, 0].
Also move _build_interval from trig.jl to the main module so arithmetic.jl
can use it, and add sign_rev tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
src/decorated.jl(referencesDecoratedIntervalwhich no longer exists in IntervalArithmetic v1),src/pave.jl(pre-Julia-1.0 syntax),src/powers.jl(square!/cube!were unused and thecube_posimplementation is explicitly marked "not rigorous"),examples/examples.jl(references a non-existentsquarefunction), and a straytest/Non1788tests/script.jl. Also remove unused mutating helpersexp!,log!,asin!.asin_revwas ignoring its inputxargument. Example:asin_rev([0,1], [0,0.5])[2]returned[0, 0.841]; now correctly returns[0, 0.5].max_rev/min_revnow return all-empty whena,b, orcis empty, and guardinterval(lo, hi)construction againstlo > hivia a small helper. Removes ~9 of the ~10 ill-formed-interval warnings that test runs were emitting.sign_rev. The previous body was commented out with broken syntax while still exported. New implementation:sign(x) ∈ {-1, 0, 1}, so the preimage ofais the union of half-lines /{0}depending on which of those values lie ina.cosh_rev/tanh_revdocstrings previously said "Reverse square root"; add defaultx = entireinterval(b/a)topow_rev1/pow_rev2, matching what their docstrings already claimed.Net: +111 / −289 lines across 14 files (5 deleted, 1 added, rest modified).
Test plan
Pkg.test()passes: 1355 pass (was 1344), 241 broken (same baseline as before)sign_revtests added intest/Non1788tests/sign.jlcovering empty input, empty output, singleton cases (+1, -1, 0), two-value combinations, and restriction by a boundedb.min_rev/max_revempty-abehavior were updated to expectemptyinterval(); flagged in the review as incorrect expectations.Not addressed here (follow-ups)
sin!/cos!/tan!are kept becausesin_rev/cos_rev/tan_revroute through them viaperiodise. Making them private / inlining is a larger refactor.div_revdoes not yet handle0 ∈ awithextended_div(analogous to whatmul_revalready does).min_rev/max_revlogic beyond the empty-input guard wasn't rewritten; the existing branch logic is ad-hoc and doesn't achieve tight enclosures in all cases. Recommended to revisit against Goualard (2008, hal-00288457).🤖 Generated with Claude Code