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

ENH: Use SymEngine in ESPEI #212

Merged
merged 17 commits into from Feb 21, 2022

Conversation

richardotis
Copy link
Collaborator

@richardotis richardotis commented Sep 27, 2021

This PR depends on pycalphad/pycalphad#376 - tests will fail without it

Similar to the ongoing work in the above PR, this one creates a number of compatibility fixes so that ESPEI can continue to work with the fully SymEngine-based pycalphad. It goes further to eliminate SymPy as a dependency entirely by replacing all SymPy based objects with SymEngine equivalents. The test suite passes locally when used with the above pycalphad PR. There is a slight performance difference: roughly 25 seconds on this PR, versus 50 seconds running ESPEI master and pycalphad develop. I expect this difference to be more discernible in more complex cases, especially with regard to the cold start time.

As with pycalphad, there are no direct changes to user-facing APIs here, but direct manipulation of SymEngine objects will have different semantics, so code that isn't covered by the suite may have breakages. We'll need to "kick the tires" here to convince ourselves that we're getting deterministic and numerically correct results.

Note: I temporarily modified the 'pycalphad develop' test to use a commit from pycalphad/pycalphad#376 for demonstration purposes

@bocklund
Copy link
Member

bocklund commented Oct 6, 2021

@richardotis can you merge/rebase on the master branch, in particular commit 6b8bcc1 merged in from #251?

If those tests pass, parallelization with MCMC should work. Locally, they do not pass for me because SymEngine objects cannot yet be pickled, as tracked by the following issues:

and the open pull request to add support for serialization with cereal in SymEngine:

…rium thermochemical data (PhasesResearchLab#216)

- Update legends in `espei.plot` functions to be centered vertically on the right-outer edge of the plot
- Extract the plotting function for plotting predicted non-equilibrium thermochemical data (useful for repeated calculations with different sets of parameters in uncertainty propagation)
@bocklund
Copy link
Member

Locally, the master branch merges cleanly onto this one. It should just need a merge, then bump symengine version. If all goes well (as it does locally for me), then we should just need to bump the pycalphad pin when a new pycalphad version is released

@bocklund
Copy link
Member

I merged the master branch, bumped the pycalphad test hash, and added a new failing test that I discovered.

espei/paramselect.py Outdated Show resolved Hide resolved
@bocklund bocklund marked this pull request as ready for review February 21, 2022 02:21
@bocklund bocklund self-requested a review February 21, 2022 02:21
@bocklund bocklund merged commit b9bc9fe into PhasesResearchLab:master Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants