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

Add gradient to linear interpolant #68

Merged
merged 6 commits into from
Jul 8, 2022
Merged

Add gradient to linear interpolant #68

merged 6 commits into from
Jul 8, 2022

Conversation

EricWay1024
Copy link
Collaborator

#27

@@ -67,6 +67,27 @@ def __call__(self, x, y):
# Final result
return np.where(f1 == f2, f1, (y2 - y) * f1 + (y - y1) * f2)

def grad(self, x, y):
Copy link
Member

Choose a reason for hiding this comment

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

Should we make it so that this returns the value and the gradient? I can't think of many scenarios where you just want the gradient, and the functions overlap quite a bit

Copy link
Member

Choose a reason for hiding this comment

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

Could you also explain the returned format in the docstring? Maybe give an example too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see... So is it better (and what you originally meant) to incorporate grad as an optional parameter of the function, which makes the function return extra fields when set True?

Copy link
Member

Choose a reason for hiding this comment

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

You mean like f(x, y, True) would give you gradients?
Yeah we could do that!
Or I suppose we could write an entirely separate function (or callable class) ?
Up to you!

Copy link
Member

Choose a reason for hiding this comment

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

In PINTS we have a __call__ for just f, and then a second function for f+df/dx: https://github.com/pints-team/pints/blob/master/pints/_error_measures.py#L12-L42

There's probably no right or wrong, just pick whatever you think is easiest!

Copy link
Member

Choose a reason for hiding this comment

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

What about just outside, e.g. (-1, 0) ?

Copy link
Member

Choose a reason for hiding this comment

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

BTW, do we need to make the function value a float?

Not sure what you mean? Python 3 does this automatically if needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Like -17.86381531 instead of array(-17.86381531). I encountered an error causes by this type issue when using NLopt (which is not Python implemented).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What about just outside, e.g. (-1, 0) ?

Same result :-)

Copy link
Member

Choose a reason for hiding this comment

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

Aha! Hadn't noticed that! Yeah make it a float

@EricWay1024 EricWay1024 merged commit 79d694e into main Jul 8, 2022
@EricWay1024 EricWay1024 deleted the 27-gradient branch July 8, 2022 10:56
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