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

[WIP] performance optimizations #83

Closed
wants to merge 2 commits into from
Closed

[WIP] performance optimizations #83

wants to merge 2 commits into from

Conversation

tjhei
Copy link
Member

@tjhei tjhei commented Dec 13, 2014

No description provided.

@tjhei
Copy link
Member Author

tjhei commented Dec 13, 2014

Next step on the list is:

  • optimizing slb
  • disabling gibbs energy if not needed

@ian-r-rose I created a branch where I pushed the somewhat messy current status. Maybe you can take a look.

Only SLB is working for now.
@tjhei
Copy link
Member Author

tjhei commented Dec 15, 2014

@ian-r-rose Can you take a look at the new interface for EOS. Only SLB is (kind of?) working for now.


if self.method is None:
raise AttributeError, "no method set for mineral, or equation_of_state given in mineral.params"

self.V = self.method.volume(self.pressure, self.temperature, self.params)
self.gr = self.K_T = self.K_S = self.G = self.C_v = self.C_p = self.alpha = None
self.gibbs = self.helmholtz = self.S = self.H = None

Copy link
Contributor

Choose a reason for hiding this comment

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

I found this confusing at first (setting all of them to None, that is). Kind of ugly, but it works. No real reason not to use it, though.

@ian-r-rose
Copy link
Contributor

Overall, the interface to for EquationOfState is now more confusing, but I agree that it is for a good reason, and I do not necessarily see a better way of doing it at the moment.
As long as the new interface is documented (and I think that a few lines describing why it is designed that way would help), it should be fine.

@ian-r-rose
Copy link
Contributor

I assume that when you profiled the number of redundant calls have been drastically reduced?

@ian-r-rose
Copy link
Contributor

One thing that I would add: If we do use this interface for the equations of state, I see no reason why volume should have different arguments than every other. That is to say, why not just set that to None in set_state() as well? Then just evaluate it in molar_volume?

@bobmyhill
Copy link
Member

I think this PR is obsolete now. Can it be closed?

@tjhei
Copy link
Member Author

tjhei commented Dec 18, 2015

obsolete, but I would like to look at some of the jit functions and implement this.

@tjhei
Copy link
Member Author

tjhei commented Dec 28, 2015

see #222

@tjhei tjhei closed this Dec 28, 2015
@tjhei tjhei deleted the jit_optimizations branch January 5, 2016 14:58
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