Ability to check if bridgestan sources are already available

roualdes / bridgestan

BridgeStan provides efficient in-memory access through Python, Julia, and R to the methods of a Stan model.

https://roualdes.github.io/bridgestan

BSD 3-Clause "New" or "Revised" License

87 stars 12 forks source link

Ability to check if bridgestan sources are already available #220

Closed avehtari closed 2 weeks ago

avehtari commented 3 months ago

Why get_bridgestan_path() is not exported?
The behavior is strange. I have not set BRIDGESTAN environment variable, and get message
```
BridgeStan not found at location specified by $BRIDGESTAN environment variable, downloading version 2.4.0 to  /u/77/ave/unix/.bridgestan/bridgestan-2.4.0"
```
but /u/77/ave/unix/.bridgestan/ directory does already have working bridgestan installation, so it was able to find the correct directory, but not able to check if there is already an installation, and it goes and downloads something without giving a change to set the path if I would use some other path than the default.

WardBrian commented 3 months ago

The message here is probably a bit misleading, because it doesn't just look for a BridgeStan, it looks for a BridgeStan instance of the matching version if $BRIDGESTAN is not set manually, so it will download a new one if you update your install

get_bridgestan_path() is not really set up to be used in this way -- it is the function that triggers the download when it is not found, so it wouldn't really help here. We can think of alternatives to this design

avehtari commented 3 months ago

I was expecting the same behavior as in cmdstanr::cmdstan_path() https://mc-stan.org/cmdstanr/reference/set_cmdstan_path.html

WardBrian commented 3 months ago

We have an intentionally different behavior than cmdstan, where the get_bridgestan_path() function always returns a path, downloading it if necessary. This is in part because a given version of bridgestan is only ever really intended to work with a single version of the C++ sources, so we know which version we should download. CmdStanR/Py don't really have that notion

avehtari commented 3 months ago

Ah, that is not clear from the documentation, which is not mentioning get_bridgestan_path() at all, but advises to make the download separately, and when I use the proposed git clone option, I wanted to check whether the path points to a correct directory, I tried using get_bridgestan_path(), but then it downloaded the latest release and missed that I had the latest version via git.

WardBrian commented 3 months ago

Oh, that's just because I forgot to update the docs after the downloading feature was ported to R in #172.

Here's what we say in Python, which is now true for R as well:

The first time you compile a model, the BridgeStan source code will be downloaded and placed in ~/.bridgestan/. If you prefer to use a source distribution of BridgeStan, consult the following section. ... To use the BridgeStan source you’ve manually downloaded instead of one the package will download for you, you must use set_bridgestan_path() or the $BRIDGESTAN environment variable.

I will update R's page to say something similar

WardBrian commented 3 months ago

@avehtari I've opened a PR with doc changes -- are you also hoping for any behavior changes, or is just properly explaining the behavior enough to close this?

avehtari commented 3 months ago

Function like bridgestan_path() similar to cmdstan_path() would be helpful to check that the environment variable BRIDGESTAN is working. If setting that environment variable did not work for some reason, another (unnecessary) installation is made to ~/.bridgestan.

WardBrian commented 2 months ago

@avehtari would changing the definition of get_bridgestan_path() to accept a download argument (default TRUE) line up with what you were expecting?

avehtari commented 1 month ago

Sounds like yes