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

Fix log density spline extrapolation #133

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

msricher
Copy link
Collaborator

@msricher msricher commented Dec 8, 2024

No description provided.

Copy link
Member

@PaulWAyers PaulWAyers left a comment

Choose a reason for hiding this comment

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

Can you add some comments on what the code is doing, and how it is resolving the issue? My understanding is that you clip the vector, and then "fix" the NaN's later.

I think like 191 is just fixing a different bug.

I'm not sure what 195 is doing.

Copy link
Member

@PaulWAyers PaulWAyers left a comment

Choose a reason for hiding this comment

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

Looks good to me. @marco-2023 and/or @gabrielasd ?

np.log(y) if log else y,
# Clip y values to >=ε^2 if using log because they have to be above 0;
# having them be at least ε^2 seems to work based on my testing
np.log(y.clip(min=np.finfo(float).eps ** 2)) if log else y,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@PaulWAyers and @msricher I have a doubt, for all the features that we describe as splines, for values of r outside our array of values, the spline is a polynomial that will diverge for $r \to \infty$. If we know the correct decay for some of the features, why not use a switch function (like hyperbolic tangent) to change between the spline and the function (or even a constant value) outside its range?

@msricher msricher merged commit b742618 into theochem:master Dec 17, 2024
9 checks passed
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