jfischoff / hs-cmdstan

Haskell CmdStan Wrapper
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Warmup samples, exe paths and parameter parsing. #3

Open adamConnerSax opened 4 years ago

adamConnerSax commented 4 years ago

Thought I'd open a thread here. Easier than twitter!

I've forked and have a new version with two primary changes

  1. I added numWarmup to StanExeConfig (and various required downstream changes)

  2. I added exePath to StanSummaryConfig to make it easier to specify where that is. This could default to "CMDSTAN_DIR/bin/stansummary" which I think would be consistent with how you approach make. But then the function to make a StanSummaryConfig would become monadic like the one for make. No big deal, I think. But I though I should ask since that's a breaking change.

Two questions:

  1. That 2nd thing, the path, brings up my general confusion about the way all the paths should work and do work in Haskell. In general this all ought to be easy to customize without a complex setup from a user. Preferably just build CmdStan someplace, tell the app where, and everything works. But there's also a need to get some path or env variable right for CLANG so that make will work, right? I wonder if this should all be a Dhall config or something?

  2. I'm wondering if you'd be willing to accept a more complex type for paramStats that handled at least array parameters and maybe matrix ones via an ADT like

    data ParameterStatistics = ScalarStatistic StanStatistic 
                                            | ArrayStatistics (Vector StanStatistic) 
                                            | MatrixStatistics (HMatrix.Matrix StanStatistic)
    ...
    paramStats :: Map String ParameterStatistics
    ...

    I haven't used a matrix parameter yet but I can imagine using one and it'd be nice to have the summary be easier to pull back into Haskell. But this would all require a bit more parsing.

Thoughts?

And would you prefer the PR with just the first two sometime soon?

adamConnerSax commented 4 years ago

Actually, I think I'll just handle the parsing to arrays, etc. in a later step. When it's done you can decide if it's worth including as another module. I went a little type-happy...

jfischoff commented 4 years ago

Actually, I think I'll just handle the parsing to arrays, etc. in a later step. When it's done you can decide if it's worth including as another module. I went a little type-happy...

Yeah I'm thinking of this as the base package that provides the lowest level of functionality for wrapping Stan. There should be higher level libraries that work on top of it that provide more features and provide more type safety.

jfischoff commented 4 years ago

That 2nd thing, the path, brings up my general confusion about the way all the paths should work and do work in Haskell. In general this all ought to be easy to customize without a complex setup from a user. Preferably just build CmdStan someplace, tell the app where, and everything works. But there's also a need to get some path or env variable right for CLANG so that make will work, right? I wonder if this should all be a Dhall config or something?

I'm assuming the user has cmdstan configured properly somehow so I consider this out of scope for the moment.

jfischoff commented 4 years ago

I added numWarmup to StanExeConfig (and various required downstream changes) 👍

I added exePath to StanSummaryConfig to make it easier to specify where that is. This could default to "CMDSTAN_DIR/bin/stansummary" which I think would be consistent with how you approach make. But then the function to make a StanSummaryConfig would become monadic like the one for make. No big deal, I think. But I though I should ask since that's a breaking change.

For now I'm just assuming it must be on the path. It could be configured explicitly or the user can just do PATH=STAN_SUMMARY_DIR:$PATH there_hs_cmd_exe and it would work as well.