search

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

TBB path issue on Windows #148

Closed aseyboldt closed 1 year ago

aseyboldt commented 1 year ago

Since the last release my CI tests for nutpie fail on windows. If my remote debugging is right, this is due to the changes introduced in #118. This adds some code that is executed in __init__, ie right at import time, where it calls os.add_dll_directory with contents of the ~/.bridgestan directory. But if this runs on a fresh install, where stan was not downloaded already, this fails with a FileNotFound error.

Some logs from the CI:

https://github.com/pymc-devs/nutpie/actions/runs/5419095619/jobs/9851843966#step:8:335

If my interpretation of this is correct, this could be fixed by delaying the call to _windows_path_setup until after bridgestan is downloaded, ie by setting a flag to False at import time, and checking that flag each time a model is compiled. (strictly speaking it would be enough to check when a shared lib is loaded, but if, say, someone would want to load the library using a different interface, that interface would then have to do that...)

WardBrian commented 1 year ago

I believe your diagnosis is correct. I have two thoughts:

  1. Can we think of a reasonable way to test this behavior (it didn’t show up in our CI since we set $BRIDGESTAN, and I think we have to do that since the code will drift from the released versions)
  2. I think adding it at load time is the more correct thing to do, since a model will (presumably) be loaded more times than it is compiled. If you compile, close python, re open, and then load, that needs to work.

On:

but if, say, someone would want to load the library using a different interface, that interface would then have to do that...

I believe that the dll_load_path is specific to Python as an additional security feature. To my knowledge $PATH is used elsewhere, so I don’t believe this would be an issue?

aseyboldt commented 1 year ago

I believe that the dll_load_path is specific to Python as an additional security feature. To my knowledge $PATH is used elsewhere, so I don’t believe this would be an issue?

I'll try with the branch from the PR. If this is in fact an issue, I guess we could also expose the function? ie as a verify_library_path or so, that only does something on windows?.