Skip to content

Logical ops#58

Merged
mkeeter merged 18 commits intomainfrom
logical-ops
Apr 5, 2024
Merged

Logical ops#58
mkeeter merged 18 commits intomainfrom
logical-ops

Conversation

@mkeeter
Copy link
Copy Markdown
Owner

@mkeeter mkeeter commented Apr 3, 2024

This PR adds AND, OR, and NOT opcodes.

These operations are designed for basically one purpose: making conditionals of the form (cond && a) || (!cond && b). As such, they have the following semantics:

  • a && b ⇒ if a == 0.0 { a } else { b }
  • a || b ⇒ if a != 0.0 { a } else { b }

The operators are also designed to work well with tape simplification (through the TracingEvaluator): if cond is unambiguously 0.0 or unambiguously non-zero, then the conditional statement can be collapsed into either a or b.

(marked as a Draft because I still need to turn the crank on JIT implementations)

@mkeeter mkeeter mentioned this pull request Apr 3, 2024
@Wulfsta
Copy link
Copy Markdown
Contributor

Wulfsta commented Apr 4, 2024

This is quite an odd construction, but it makes sense to me. It seems like it should address the current issues with building piecewise functions, and allow users to plug up holes in functions that might otherwise be problematic. It seems like it might be worth making utility functions using these operators (if_zero_else, if_nonnegative_else, if_positive_else) and adding them directly to the library if feasible (I think they can just be written as functions in fidget::context::Context; I would be happy to contribute this as a follow up, if it is this easy and you agree it is useful), so users aren't made to rediscover these on their own.

Overall, I like this and think it solves the problem.

@mkeeter mkeeter force-pushed the logical-ops branch 2 times, most recently from e804922 to 6dc5ea7 Compare April 5, 2024 22:27
@mkeeter mkeeter marked this pull request as ready for review April 5, 2024 22:27
@mkeeter mkeeter enabled auto-merge (squash) April 5, 2024 22:34
@mkeeter mkeeter merged commit 7dbb1f2 into main Apr 5, 2024
@mkeeter mkeeter deleted the logical-ops branch April 5, 2024 22:36
@mkeeter
Copy link
Copy Markdown
Owner Author

mkeeter commented Apr 6, 2024

@Wulfsta Now that this is merged, adding helper functions to Context sounds sensible, and I'd be happy to review a PR.

@Wulfsta
Copy link
Copy Markdown
Contributor

Wulfsta commented Apr 6, 2024

I am on vacation at the moment, but would be happy to add this when I am back. It will probably be a week or so until I am able to work on this.

Edit: Got bored, wrote code.

@Wulfsta
Copy link
Copy Markdown
Contributor

Wulfsta commented Apr 7, 2024

@mkeeter I was just thinking about an edge case; what happens if both cond = 0 and a = 0 in (cond && a) || (!cond && b)? Seems like it would return b, which is a bit tricky to write a utility function around... I could write a warning into the docs about this case, but I don't see a programmatic guard for this. I may need to think about it more, but do you have any insight I am missing?

@Wulfsta
Copy link
Copy Markdown
Contributor

Wulfsta commented Apr 8, 2024

Aha! I think this can be avoided entirely with (cond && a) + (!cond && b). I simply didn’t see it at first.

@mkeeter
Copy link
Copy Markdown
Owner Author

mkeeter commented Apr 8, 2024

I was just thinking about an edge case; what happens if both cond = 0 and a = 0 in (cond && a) || (!cond && b)? Seems like it would return b, which is a bit tricky to write a utility function around...

Isn't returning b correct if cond is 0 (regardless of the value of a)?

e.g. phrasing it as a conditional

if cond:
    a
else:
    b

I would expect this to return b if cond is 0.

@Wulfsta
Copy link
Copy Markdown
Contributor

Wulfsta commented Apr 8, 2024

Continuing in #64.

mkeeter pushed a commit that referenced this pull request Apr 8, 2024
This implements an `if_nonzero_else` helper function as implied in #58.
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.

2 participants