Skip to content

Conversation

@obround
Copy link
Contributor

@obround obround commented Oct 17, 2025

I was thinking we should move ival-pow2 to core.rkt, because I feel like it fits in there more, and we can remove a bunch of exports that I added from core.rkt.

@obround obround requested a review from pavpanchekha October 17, 2025 20:48
@pavpanchekha
Copy link
Contributor

This looks fine to me. About reorganizing the files, eh. Here's what I'll say:

  • I agree that the Rival codebase is a bit of a mess. eval/ is OK but ops.rkt once upon a time was a single file in the Herbie repo, and while I've moved some stuff out of core.rkt it's still a big mess. (infra/ is a really big mess, too, but that's a separate thing.)
  • pow2 specifically could be in pow.rkt or arith.rkt or core.rkt, it's really very simple
  • That said, I'd rather move stuff out of core.rkt rather than intocore.rkt. Like, I wouldn't mind moving the comparison stuff out, and the monotonic stuff. atan2 should be its own file. If we do that, then core.rkt would be just core data structures and helper functions, which might be nice. I'd also love to cut down the number of helper functions; adding more mutable stuff will do that automatically, other stuff is a bit trickier (epbinary and endpoint-min2/max2 overlap in practice but just replacing one with the other doesn't work, you gotta think about rounding carefully.)
  • I think the immutable stuff, and some legacy functions like close-enough->ival should also live in their own file; eventually I think the plan is for Rival not to use those, only the mutable versions, and immutable APIs are just provided for compatibility / convenience if anyone needs an interval library.

You can spend forever organizing stuff so I wouldn't hold up a merge either way. Usually it's best to do a "big bang" re-organization, because a big reorg hits every other PR. So if you want to do a lot of organizing, I'd say write up a little Google Doc proposal that we can discuss in meeting and then can just do in one big step.

@obround
Copy link
Contributor Author

obround commented Oct 17, 2025

Alright, cool, I think I'll put together a small Google Doc proposal.

@obround obround merged commit 6699412 into main Oct 17, 2025
1 check passed
@obround obround deleted the direct-rounding-pow branch October 17, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants