Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

outdated ptemcee fail tests #114

Open
mjwen opened this issue Mar 28, 2023 · 1 comment
Open

outdated ptemcee fail tests #114

mjwen opened this issue Mar 28, 2023 · 1 comment

Comments

@mjwen
Copy link
Collaborator

mjwen commented Mar 28, 2023

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
Copy link
Contributor

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants