ADVI can write 1.#INF on Windows, which cannot be read

alashworth commented 5 years ago

Issue by bgoodri Tuesday Apr 18, 2017 at 23:39 GMT Originally opened as https://github.com/stan-dev/stan/issues/2283

Summary:

It is possible for ADVI to obtain a realization that is infinite, and if it writes that to a CSV file, reading the CSV file breaks. We should coerce this to the nearest floating point number before writing.

Description:

CSV files should only have finite numerical values below the headings

Reproducible Steps:

In R 3.2.x for Windows, load rstanarm 2.15.x and do

fit <- stan_glm(mpg ~ ., data = mtcars, prior = hs_plus(4, 1, 2, 0.5), 
                       seed = 12345 algorithm = "meanfield", QR = TRUE)

Current Output:

scan() expected 'a real', got '1.#INF'

Expected Output:

Finite estimates

Additional Information:

Provide any additional information here.

Current Version:

v2.15.0

alashworth commented 5 years ago

Comment by bob-carpenter Wednesday Apr 19, 2017 at 20:11 GMT

Is there something in this (stan-dev/stan) repo that should be fixed?

Would it be OK if that fix allowed infinite and NaN values to be read?

I added the next release as milestone and assigned to @mitzimorris but I'm still not sure what needs to be fixed (if anything) in this repo.

alashworth commented 5 years ago

Comment by syclik Wednesday Apr 19, 2017 at 20:41 GMT

Hmm. This is in the writers. Currently, we use stringstream's operator<< to take a double and get it out to a string. The issue here is that Windows prints infinity as #1.INF and NaN as #1.NAN while Linux and Mac does something differently.

Some reasonable options to fix this:

In Stan, update the writers so that it prints something not OS dependent. That's this file: src/stan/callbacks/stream_writer.hpp
Update RStan's writer to write something different when it sees NaN or Inf. We'd want to do the same for CmdStan and PyStan, I think.

On Wed, Apr 19, 2017 at 4:11 PM, Bob Carpenter notifications@github.com wrote:

Is there something in this (stan-dev/stan) repo that should be fixed?

Would it be OK if that fix allowed infinite and NaN values to be read?

I added the next release as milestone and assigned to @mitzimorris https://github.com/mitzimorris but I'm still not sure what needs to be fixed (if anything) in this repo.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stan-dev/stan/issues/2283#issuecomment-295420214, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZ_F5ZAUqUroVQUw04fq2cmtFc0IHgNks5rxmqAgaJpZM4NBBmK .

alashworth commented 5 years ago

Comment by syclik Tuesday May 09, 2017 at 21:22 GMT

@bgoodri, thoughts? Can we either close this or list what needs to be done?

alashworth commented 5 years ago

Comment by bgoodri Tuesday May 09, 2017 at 21:30 GMT

We should fix ADVI in Stan so that it does not report values greater than the largest floating point number.

On Tue, May 9, 2017 at 5:22 PM, Daniel Lee notifications@github.com wrote:

@bgoodri https://github.com/bgoodri, thoughts? Can we either close this or list what needs to be done?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stan-dev/stan/issues/2283#issuecomment-300305479, or mute the thread https://github.com/notifications/unsubscribe-auth/ADOrqjiq_rFELMQyRPjbTCwEuqqiTGEbks5r4NkJgaJpZM4NBBmK .

alashworth commented 5 years ago

Comment by syclik Tuesday May 09, 2017 at 21:54 GMT

That seems reasonable. Can you either update this issue to reflect that our close this and open a new one with that suggestion?

On May 9, 2017, at 5:30 PM, bgoodri notifications@github.com wrote:

We should fix ADVI in Stan so that it does not report values greater than the largest floating point number.

On Tue, May 9, 2017 at 5:22 PM, Daniel Lee notifications@github.com wrote:

@bgoodri https://github.com/bgoodri, thoughts? Can we either close this or list what needs to be done?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stan-dev/stan/issues/2283#issuecomment-300305479, or mute the thread https://github.com/notifications/unsubscribe-auth/ADOrqjiq_rFELMQyRPjbTCwEuqqiTGEbks5r4NkJgaJpZM4NBBmK .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

alashworth commented 5 years ago

Comment by bob-carpenter Wednesday May 10, 2017 at 18:50 GMT

@bgoodri: is your proposal todo censoring and report the max double instead of +infinity?

Is there no way to robustly read NaN or infinite values into R? Because I'd think the right fix would be fixing our writers to match whatever that format is.

alashworth commented 5 years ago

Comment by bgoodri Wednesday May 10, 2017 at 21:38 GMT

Yes, and not that I know of.

On Wed, May 10, 2017 at 2:50 PM, Bob Carpenter notifications@github.com wrote:

@bgoodri: is your proposal todo censoring and report the max double instead of +infinity?

Is there no way to robustly read NaN or infinite values into R? Because I'd think the right fix would be fixing our writers to match whatever that format is.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stan-dev/stan/issues/2283#issuecomment-300578384, or mute the thread https://github.com/notifications/unsubscribe-auth/ADOrqj5zAPVfrv_SRJrQnEepBKptH3wEks5r4gbpgaJpZM4NBBmK .

alashworth commented 5 years ago

Comment by jgabry Wednesday May 10, 2017 at 21:47 GMT

I'm not aware of a super robust method for reading them into R either, unfortunately.

On Wed, May 10, 2017 at 5:38 PM bgoodri notifications@github.com wrote:

Yes, and not that I know of.

On Wed, May 10, 2017 at 2:50 PM, Bob Carpenter notifications@github.com wrote:

@bgoodri: is your proposal todo censoring and report the max double instead of +infinity?

Is there no way to robustly read NaN or infinite values into R? Because I'd think the right fix would be fixing our writers to match whatever that format is.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stan-dev/stan/issues/2283#issuecomment-300578384, or mute the thread < https://github.com/notifications/unsubscribe-auth/ADOrqj5zAPVfrv_SRJrQnEepBKptH3wEks5r4gbpgaJpZM4NBBmK

.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stan-dev/stan/issues/2283#issuecomment-300620287, or mute the thread https://github.com/notifications/unsubscribe-auth/AHb4QzrHgQYnoyx0Qut3lcXJcP6t8pOyks5r4i5SgaJpZM4NBBmK .

alashworth commented 5 years ago

Comment by mitzimorris Friday May 12, 2017 at 13:21 GMT

what is the current proposal? initial proposal was this:

In Stan, update the writers so that it prints something not OS dependent. That's this file: src/stan/callbacks/stream_writer.hpp

unassigned myself per subsequent discussion, but happy to do this if that's what we want.

alashworth commented 5 years ago

Comment by ahartikainen Monday Jun 12, 2017 at 12:04 GMT

Are these not the common/preferred ways to write&read infinities / nans to and from CSV?

They should be platform independent. I mean reading with common R/Python(Pandas) tools.

inf
-inf
nan or " "

alashworth / test-issue-import