Mosfit not fully compatible with astropy v5

[robertdstein]

Some features of Mosfit are not compatible with astropy 5, because astropy Quantity values are no longer directly compatible with numpy.logspace (see e.g https://github.com/astropy/astropy/issues/10573).

A minimal example, with astropy==4.3.1 and numpy=1.22:

mosfit -m slsn

which runs without errors. However, repeating the same command with astropy==5.0 and numpy=1.22 yields:

  File "/home/rdstein/.conda/envs/tdescore_env/bin/mosfit", line 8, in <module>
    sys.exit(main())
  File "/home/rdstein/.conda/envs/tdescore_env/lib/python3.8/site-packages/mosfit/main.py", line 973, in main
    Fitter(**fitargs).fit_events(**fitargs)
  File "/home/rdstein/.conda/envs/tdescore_env/lib/python3.8/site-packages/mosfit/fitter.py", line 477, in fit_events
    entry, p, lnprob = self.fit_data(
  File "/home/rdstein/.conda/envs/tdescore_env/lib/python3.8/site-packages/mosfit/fitter.py", line 686, in fit_data
    output = model.run_stack(x, root='output')
  File "/home/rdstein/.conda/envs/tdescore_env/lib/python3.8/site-packages/mosfit/model.py", line 970, in run_stack
    new_outs = self._modules[task].process(**inputs)
  File "/data/rdstein/.conda/envs/tdescore_env/lib/python3.8/site-packages/mosfit/modules/arrays/densetimes.py", line 36, in process
    x + self._rest_t_explosion for x in np.logspace(
  File "<__array_function__ internals>", line 180, in logspace
TypeError: no implementation found for 'numpy.logspace' on types that implement __array_function__: [<class 'astropy.units.quantity.Quantity'>]

I am pretty sure this is the same problem problem described in #207.

I think this problem would be rather complex to solve. It would involve ensuring that all variables passed in Mosfit are either always an astropy quantity or always a numpy float/int etc. Right now some variables can swap between the categories. An example is self._rest_t_explosion in densetimes.py, which causes the error above and is both an astropy quantity and numpy float at different points during the call of mosfit -m slsn. If the variables were reliably in one class or the other, the astropy quantities would then need to be converted each time np.logspace are used.

For https://github.com/guillochon/MOSFiT/blob/b9d1895aef54e06669fba09e11248f8d2f65777f/mosfit/modules/arrays/densetimes.py#L38 you can't simply replace with np.log10(max_times - self._rest_t_explosion.value) because if self._rest_t_explosion is not an astropy quantity this will throw an error.

[mnicholl]

I've finally got to testing this, but I can't reproduce this error or the related #207. Which version of Python are you using? If you try with Python 3.10, astropy 5.1 and numpy 1.23, do you still get an error?

[robertdstein]

I tried with the latest github version of Mosfit (1.1.8), python 3.10.4, astropy 5.1, numpy 1.23.3, and I do still get the error. For reference this is on a Mac M1 (2021).

I then tried the a clean install on a random linux machine:

1. create a new conda environment, and activate it
2. conda install -c conda-forge mpi4py
3. pip install mosfit
4. mosfit -m slsn

and there I get exactly the same error. So it does not seem to be platform-specific for me. Anything obviously wrong there?

[pkgw]

We're now seeing this error in the conda-forge feedstock build process: https://github.com/conda-forge/mosfit-feedstock/pull/50 . Current builds are with Astropy 5.1.1, Numpy 1.23.4, across Python versions.

[mnicholl]

I believe I have now fixed this!

I'm bumping the version number so we can update the distributions. I've done this on pypi -- Peter would you be able to do it on conda-forge? I'm not sure of the process for that.

Would also be great to get confirmation that this is working for others

[pkgw]

@mnicholl Great! The conda-forge update should be close to automatic — we have bots that look for new releases and automatically submit pull requests to initiate the updates. I'll ping you if anything comes up but you basically shouldn't have to worry about that part of the process.

[robertdstein]

I confirm that I just tested this with astropy 5.1.1. The error is reproduced for mosfit 1.1.8, but is completely solved in 1.1.9, so it looks like the fix works. Thanks @mnicholl!

[pkgw]

As seen in https://github.com/conda-forge/mosfit-feedstock/pull/51, the new release builds successfully in conda-forge too. I think it's safe to mark this one as solved.