Skip to content

Should NaN's display as errors? #8

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

Closed
penelopeysm opened this issue Apr 10, 2025 · 2 comments · Fixed by #13
Closed

Should NaN's display as errors? #8

penelopeysm opened this issue Apr 10, 2025 · 2 comments · Fixed by #13
Labels
this-repo Something to do with just this repo

Comments

@penelopeysm
Copy link
Member

Some of the FiniteDifferences 'wrong' entries are not really numerically wrong, but rather NaN's.

It appears to me that it would be fairer to mark NaN as 'error' rather than 'wrong', since attempting to use this in sampling will fail (rather than proceeding silently with the wrong result, which is the scenario we really want to be careful about).

@penelopeysm penelopeysm added the this-repo Something to do with just this repo label Apr 10, 2025
@yebai
Copy link
Member

yebai commented Apr 10, 2025

I suggest we distinguish between two types of wrongs

  • Wrong (incorrect answer)
  • Wrong (NaN)

@penelopeysm
Copy link
Member Author

penelopeysm commented Apr 21, 2025

Distinguishing between outright incorrect answers and NaN's will be possible once DynamicPPL 0.36 (and specifically this PR TuringLang/DynamicPPL.jl#890) is released!

Not closing yet as there is still some work to be done in this repo, although it will just be a fairly trivial any(isnan) check.

(Incidentally, that release will also make the AD run with linked varinfo by default, so it should actually get rid of the NaN's)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
this-repo Something to do with just this repo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants