Open MJKirk opened 4 months ago
Hmm, I think the first three get_
functions in ultranest.hotstart could import scipy.stats.t when called. I'd rather not have scipy as a hard requirement, only when some of its functions are needed (integrator.py does not need it).
Similarly for hdf5.
hdf5 and scipy, being binary libraries, can be quite difficult to install on some legacy systems. Some users may be happy with non-storage functionality.
By the way, the conda package does have scipy as a run requirement, since conda can ensure a scipy install (pip not necessarily). https://github.com/conda-forge/ultranest-feedstock/blob/main/recipe/meta.yaml
Fair enough, I understand not wanting scipy and hdf5 as default requirements for those reasons. What do you think about switching the default storage backend away from hdf5 then though?
The default is to not store, you have to opt in to storing by setting log_dir.
Yes, my point was just that once you do switch on logging by setting that option, a "standard" user will likely immediately get an error, since the code uses HDF5 by default but this is not installed with ultranest. It just seems like not very friendly user behaviour to me. But I suppose it's not a big issue, as the error they get is relatively obvious to interpret.
Without scipy installed, if you
pip install ultranest
and then try and import it, it fails sinceultranest/hotstart.py
is imported (via the__init__.py
and thenultranest/integrator.py
) and thehotstart.py
file tries to import scipy: https://github.com/JohannesBuchner/UltraNest/blob/bceff1a62dc27c40b2a32f94ebd276b791eca76f/ultranest/hotstart.py#L14 So scipy should be added tosetup.py
as a requirement.In addition, if you enable logging via the
log_dir
argument, the default is to use HDF5 as the output format https://github.com/JohannesBuchner/UltraNest/blob/bceff1a62dc27c40b2a32f94ebd276b791eca76f/ultranest/integrator.py#L1045 so you can quickly get another failure if you don't have h5py installed. I would say it therefore makes sense to either also addh5py
as a requirement, or switch the default to csv/tsv.Happy to make a PR if you agree