search

openkim / kliff

KIM-based Learning-Integrated Fitting Framework for interatomic potentials.
https://kliff.readthedocs.io
GNU Lesser General Public License v2.1
34 stars 20 forks source link

outdated ptemcee fail tests #114

Open mjwen opened 1 year ago

mjwen commented 1 year ago

Seems nobody is maintaining ptemcee... and it hasn't been updated for a while.

Now, it causes numpy problems: it uses np.float, but this is removed in the newer versions of numpy. For example, see the below line (line 28)

self.nswap = np.zeros(self.ntemps, dtype=np.float)

in the CI test https://github.com/openkim/kliff/actions/runs/4486429980/jobs/7888910494

I believe we will continue using it. Here are some options:

  1. make PR in ptemcee (preferred)
  2. fork it and we make our forked version a dependence of kliff (if it is not possible to get PR merged to ptemcee)
  3. we do not test ptemcee on CI (not preferred, but acceptable). I've tried this, but we need to make some modifications to our UQ codes to make it happen. This is because MCMC is imported here https://github.com/openkim/kliff/blob/master/kliff/uq/__init__.py, which will lead to error because when we do pytest, it will first try to import all modules.

@yonatank93 any idea on this?

yonatank93 commented 1 year ago

@mjwen Thanks for bringing this up! I actually noticed that the repo hasn't been maintained for a long time. However, it had not caused any major issues for me, so I was ok with it.

Seems nobody is maintaining ptemcee... and it hasn't been updated for a while.

Now, it causes numpy problems: it uses np.float, but this is removed in the newer versions of numpy. For example, see the below line (line 28)

self.nswap = np.zeros(self.ntemps, dtype=np.float)

in the CI test https://github.com/openkim/kliff/actions/runs/4486429980/jobs/7888910494

I believe we will continue using it. Here are some options:

  1. make PR in ptemcee (preferred)

Although this is a preferred option, I am not sure if we will get far with this. I noticed that there are many PR in ptemcee from long ago, and they are still there. So, I don't think if we create a PR, the owner will review it. We can give it a shot, but we should also have a backup plan, for example with your second option.

  1. fork it and we make our forked version a dependence of kliff (if it is not possible to get PR merged to ptemcee)

Personally, seeing how ptemcee is maintained (or rather not maintained), I think this option would be the one to go. However, again, we will still try to merge the change to the original repo.

Are you thinking to fork ptemcee to openkim? I don't think I have access to fork ptemcee to openkim repo. If you can do this, I can work on updating the repo afterward.

  1. we do not test ptemcee on CI (not preferred, but acceptable). I've tried this, but we need to make some modifications to our UQ codes to make it happen. This is because MCMC is imported here https://github.com/openkim/kliff/blob/master/kliff/uq/__init__.py, which will lead to error because when we do pytest, it will first try to import all modules.

I will go with option 2. The only case that we go to this option is if we want to have my changes merged before Mach conference. Although it would be nice, I think we should not rush it. I will still go to option 2.

@yonatank93 any idea on this?