Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ability to define and query function inverses #1338

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

AayushSabharwal
Copy link
Contributor

Close #1337

I'd like input on the NaNMath inverses. They violate the contract that inverse(inverse(f)) == f. We could leave it be, define wrapper functions, or add to the macro the ability to wrap the provided inverse function in an anonymous function. For example,

@register_inverse f g wrap_inverse = true

would define

var"some_gensymed_name"(x) = g(x)
inverse(::typeof(f)) = var"some_gensymed_name"
inverse(::typeof(var"some_gensymed_name")) = f

@codecov-commenter
Copy link

codecov-commenter commented Nov 3, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 45.71429% with 19 lines in your changes missing coverage. Please review.

Project coverage is 77.77%. Comparing base (3e31a75) to head (affad5d).
Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
src/inverse.jl 45.71% 19 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1338       +/-   ##
===========================================
+ Coverage    6.69%   77.77%   +71.08%     
===========================================
  Files          47       49        +2     
  Lines        4618     4801      +183     
===========================================
+ Hits          309     3734     +3425     
+ Misses       4309     1067     -3242     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +98 to +112
"""
$(TYPEDSIGNATURES)

A simple utility function which returns the square of the input. Used to define
the inverse of `sqrt`.
"""
square(x) = x ^ 2

"""
$(TYPEDSIGNATURES)

A simple utility function which returns the cube of the input. Used to define
the inverse of `cbrt`.
"""
cube(x) = x ^ 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we should lowering sqrt to Pow

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but why would that preclude defining the inverse?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case we wouldn't need to, since the function would not be in the representation.

Comment on lines +18 to +19
`right_inverse` and `inverse` must not be defined and should error. `right_inverse(g)`
should also be defined to return `f`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

@AayushSabharwal AayushSabharwal Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If f: X -> Y is an injective function and g: Y -> X is its left inverse, then I think it can be proven that g is surjective and has f as (one of) its right inverse(s) (and vice versa)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, never thought about that.

@ChrisRackauckas ChrisRackauckas merged commit ee91afd into JuliaSymbolics:master Nov 4, 2024
10 of 12 checks passed
@AayushSabharwal AayushSabharwal deleted the as/inverse branch November 4, 2024 08:35
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.

RFC: Defining inverse functions
3 participants