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

use aesara and pymc4 instead of theano and pymc3 #706

Closed
wants to merge 15 commits into from

Conversation

AndrewAnnex
Copy link
Member

…No clue if CI builds would work. todo need to update the github worker to allow this branch to run.

likely there are a lot of api breaking changes I did not address here, I did not attempt to run it locally yet. Also the CI environment for gempy is a bit confusing (still using travis?). Likely these scripts could be cleaned up a bit more to make testing easier

Description

swap out all usage of theano for aesara and pymc3 for pymc4.

Relates to
#701

Checklist

  • My code follows the PEP 8 style guidelines.
  • My code uses type hinting for function and method arguments and return values.
  • My code contains descriptive and helpful docstrings
    which are formatted per the Google Python Style Guidelines.
  • I have created tests which entirely cover my code.
  • The test code either 1. demonstrates at least one valuable use case (e.g. integration tests)
    or 2. verifies that outputs are as expected for given inputs (e.g. unit tests).
  • New and existing tests pass locally with my changes.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@AndrewAnnex AndrewAnnex changed the title start of find and replace, may have renamed things too aggressively. … use aesara and pymc4 instead of theano and pymc3 Jun 28, 2022
@AndrewAnnex AndrewAnnex mentioned this pull request Jul 1, 2022
@Japhiolite
Copy link
Collaborator

The checks were always failing at the installation step. Apparently with aesara 2.7.8 there'd been a change with setuptools (aesara-devs/aesara#1050). Out of curiosity, I temporary froze aesara to 2.7.7 just to see whether the checks pass. Seems...like there are all tests passing now 🎉
Will have a look at this setuptools thing to get aesara out of the ice-box.

@AndrewAnnex
Copy link
Member Author

we should also do some benchmarks with a gpu to ensure things are still fast, can probably help with that

@AndrewAnnex
Copy link
Member Author

@Japhiolite that bug in aesara was fixed a couple of weeks ago. I can perform some benchmarking tests if needed

@Japhiolite
Copy link
Collaborator

That would be great @AndrewAnnex

I've talked to @Leguark about merging, and we thought that we publish the switch to aesara as gempy 2.3

@AlexanderJuestel AlexanderJuestel added this to the GemPy 2.3 milestone Feb 22, 2023
@Japhiolite
Copy link
Collaborator

Currently, the remaining failing tests are due to a (in the meantime fixed) deprecation in subsurface. Unfortunately I can't deploy a new subsurface version to pypi.

But I was wondering though....whether we just should move forward with this PR - as is - to Version 2.3 and work from there to mainly get the pandas fixes up to speed with the new major version...with pandas 2.0 apparently coming out within the next week.

Thoughts?

@AlexanderJuestel
Copy link
Contributor

Agree with proceeding to get GemPy properly working again

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newest arviz version ( v0.15.1 ) does not have a from_pymc3() method anymore...in fact, I don't see a from_pymc() method either....So I guess we'll have to see how to fix this as we go by.

@Japhiolite Japhiolite mentioned this pull request May 15, 2023
6 tasks
@AndrewAnnex
Copy link
Member Author

closing as per #796

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

Successfully merging this pull request may close these issues.

3 participants