Added the Vinet EOS #158

CaymanUnterborn · 2015-12-11T04:35:48Z

Added the Vinet EOS from a copied birch_murnaghan.py. May need to be updated to be compatible, just let me know what needs to be done. Also included the 4th order birch murghnahan EOS and Fe minerals to use both new EOS's

sannecottaar · 2015-12-11T05:38:19Z

burnman/eos/helper.py

@@ -20,6 +21,8 @@ def create(method):
    if isinstance(method, basestring):
        if method == "slb2":
            return slb.SLB2()
+        elif method == "V":


"V" to "Vinet"

tjhei · 2015-12-11T06:12:05Z

please rebase and add unittests. /run-tests

ian-r-rose · 2015-12-13T02:10:19Z

burnman/minerals/other.py

+                'Kprime_0': 6.08,
+                'molar_mass': 0.055845,
+                'n': 1,} #number of atoms per formula unit
+        Mineral.__init__(self)


Github is complaining about no newline here?

tjhei · 2015-12-13T06:11:26Z

can you take over tjhei@8f76d18 ?

tjhei · 2015-12-15T23:17:26Z

burnman/eos/birch_murnaghan.py

+    This currently does not include shear modulus
+    """
+    def __init__(self):
+        self.order=4


please add a newline after this line

sannecottaar · 2016-01-05T08:50:38Z

Do we want to finish this for the release?

CaymanUnterborn · 2016-01-05T09:39:19Z

I think there may be a rebase issue although I'm not sure exactly what the "Merge remote-tracking branch 'geodynamics/master' into Planets"

tjhei · 2016-01-05T11:46:13Z

We'll need to send you through another git bootcamp, I guess. :-) Don't do any merge/pulls unless you really want to.

tjhei · 2016-01-05T11:46:29Z

What is the status of this, Cayman?

CaymanUnterborn · 2016-01-05T17:26:14Z

I need to split BM into two separate files and then there's nothing else to do really. Can you explain the errors the tester is spitting out Timo so I can fix them? I still can't get anything but test.py to run on my machine.

tjhei · 2016-01-05T17:50:14Z

please take over tjhei@fdcdeb2 then

tjhei · 2016-01-05T17:50:40Z

(or I'll do that when it is good to merge). Anybody else have some comments?

CaymanUnterborn · 2016-01-05T18:48:45Z

Alright, BM is split into 4th. Everything should be okay to go, tests.py passes.

ian-r-rose · 2016-01-05T18:59:56Z

burnman/eos/birch_murnaghan_4th.py

+        raise ValueError('Cannot find a volume, perhaps you are outside of the range of validity for the equation of state?')
+    return opt.brentq(func, sol[0], sol[1])
+
+def volume_fourth_order(pressure,params):


Have you experimented with how fussy brentq is for this EoS? My intuition is that it would still blow up fairly easily (as with BM3).

I wonder if using the new bracket function, as above, would be more robust.

My experiments show it's also pretty fussy, particularly with brentq. I'll switch to bracket though.

Using bracket should be less fussy (unless I've totally screwed that up). It would be neat to see an example -- perhaps a test?

I'm imagining the example will be Planet builder. I can write a test too, once I think of one...

The test would not need to be complicated, just something that instantiates
a mineral and evaluates the EoS. Extra points if it is near the edge of the
range of validity for the EoS.

On Tue, Jan 5, 2016 at 11:08 AM, Cayman Unterborn notifications@github.com
wrote:

In burnman/eos/birch_murnaghan_4th.py
#158 (comment):

return K

+def volume(pressure, params):

"""

Get the birch-murnaghan volume at a reference temperature for a given

pressure :math:[Pa]. Returns molar volume in :math:[m^3]

"""

func = lambda x: birch_murnaghan(params['V_0']/x, params) - pressure

try:

sol = bracket(func, params['V_0'], 1.e-2*params['V_0'])

except:

raise ValueError('Cannot find a volume, perhaps you are outside of the range of validity for the equation of state?')

return opt.brentq(func, sol[0], sol[1])

+def volume_fourth_order(pressure,params):

I'm imagining the example will be Planet builder. I can write a test too,
once I think of one...

—
Reply to this email directly or view it on GitHub
https://github.com/geodynamics/burnman/pull/158/files#r48882813.

Don't we have a test that does this already? I can write a "high_P" test that will do this although I'm not sure what that would tell us

I suppose test_minerals.py does to that, though it does not call all the
relevant functions in the EoS.

On Tue, Jan 5, 2016 at 11:28 AM, Cayman Unterborn notifications@github.com
wrote:

In burnman/eos/birch_murnaghan_4th.py
#158 (comment):

return K

+def volume(pressure, params):

"""

Get the birch-murnaghan volume at a reference temperature for a given

pressure :math:[Pa]. Returns molar volume in :math:[m^3]

"""

func = lambda x: birch_murnaghan(params['V_0']/x, params) - pressure

try:

sol = bracket(func, params['V_0'], 1.e-2*params['V_0'])

except:

raise ValueError('Cannot find a volume, perhaps you are outside of the range of validity for the equation of state?')

return opt.brentq(func, sol[0], sol[1])

+def volume_fourth_order(pressure,params):

Don't we have a test that does this already? I can write a "high_P" test
that will do this although I'm not sure what that would tell us

—
Reply to this email directly or view it on GitHub
https://github.com/geodynamics/burnman/pull/158/files#r48885256.

I've tried adding things to test_minerals to make it work, however it seems the fluids don't have molar masses and thus won't work if I call say their V_phi's (to test for K) or rho's. Do we not have a way to call just bulk modulus?

CaymanUnterborn · 2016-01-07T19:49:07Z

Any word on this? My paper got accepted today so I'd like to include the planet builder in the release if possible.

ian-r-rose · 2016-01-07T19:52:10Z

burnman/eos/vinet.py

+        Returns shear modulus :math:`G` of the mineral. :math:`[Pa]`
+        Currently not included in the Vinet EOS, so omitted.
+        """
+        return 0


Can you make this a floating point number?

ian-r-rose · 2016-01-07T19:55:10Z

I think it is close. Can you figure out what is going on with the failing test?

tjhei · 2016-01-07T19:57:10Z

This is just the table.py output, I can fix that if you accept the PR otherwise.

tjhei · 2016-01-07T19:58:11Z

burnman/minerals/other.py

+            'molar_mass': 0.055845,
+        }
+        Mineral.__init__(self)
+class Fe_Dewaele(Mineral):


please add two empty lines between classes

I think this is all ready to go. Might need a rebase though

CaymanUnterborn · 2016-02-16T23:47:55Z

What's the word? Timo said he can fix the tests but is there anything else?

bobmyhill · 2016-02-17T00:11:13Z

There still aren't any unit tests, which would be very useful. Also, a couple of P, T points to endmember_benchmarks.py to check for implementation changes in the future (as I've done for HP, HHPH and SLB). Finally, I think that a benchmark for each of BM4 and Vinet would be good (some figure from a paper that we can use to show the EoS has been implemented correctly). We've caught problems in both HP and SLB from plotting up diagrams from the original papers.

CaymanUnterborn · 2016-02-23T00:46:22Z

What tests I could figure out have been added, feel free to suggest more. I've also added the benchmarking for both BM4 and Vinet.

tjhei · 2016-02-24T12:41:41Z

examples/example_user_input_material.py

+        (your choice in geotherm will not matter in this case)
+    or 'vinet' (vinet equation of state, if you choose to ignore temperature
+        (your choice in geotherm will not matter in this case)))"""
+    method = 'vinet'


Is there a reason you switch to "vinet" here? I assume slb3 is a better default?

I agree with Timo. slb3 should be the default - but of course vinet should be added to the list.
vinet should also be added to example_compare_all_methods.py.
I don't think there's any need to add another example... this is fine for me.

tjhei · 2016-02-24T12:43:41Z

@bobmyhill can you take another look?

tjhei · 2016-02-24T12:44:30Z

(and we need a rebase or merge from master before we can merge this, Cayman)

bobmyhill · 2016-02-24T14:57:48Z

misc/benchmarks/benchmark.py

+    plt.title("Comparing with Figure 1 of Dewaele et al., (2006)")
+
+    plt.show()
+
 def check_mgd_shim_duffy_kenichi():
    """
    Attemmpts to recreate Shim Duffy Kenichi (2002)


bobmyhill · 2016-02-24T16:58:11Z

I didn't think about it before, but birch_murnaghan_4th.py is not a very elegant way to deal with the fourth order EoS. Can you merge it into birch_murnaghan.py, making a new subclass (BM4)? The new functions look fine, so it wouldn't take very long.
Vinet looks fine.
... comments on examples/tests to follow ...

bobmyhill · 2016-02-24T17:11:28Z

tests/test_eos.py

@@ -71,6 +97,41 @@ def test_reference_values(self):
            Grun_test = i.grueneisen_parameter(pressure, temperature, rock.params['V_0'], rock.params)
            self.assertFloatEqual(Grun_test, rock.params['grueneisen_0'])

+    def test_reference_values_noG(self):


Why the different temperatures (300., 0.) in this test? The temperature shouldn't matter, surely.
If you have a reference pressure as 0. (rather than 1.e5) Pa, then this should be in the params as P_0, and you should test that the Vinet EoS handles different reference pressures in the same way that MT and BM do.

IMO, this should be made into at least two tests; one for Vinet, and one for BM4.

Another sensible (and extremely useful) test for BM4 would be to set K'' to the value inferred by BM3, and make sure that the results are the same at some high pressure. Same goes for BM3 wrt. BM2, if we don't have that test already.

CaymanUnterborn · 2016-02-24T17:15:04Z

They were originally entirely in BM, however because BM2 and BM3, however the naming convention there doesn't match what BM4 really is. Ian suggested to just split it into two different EOS's. Is that right Ian?

bobmyhill · 2016-02-24T17:25:01Z

What do you mean? BM4 is just an extension of BM3, just as BM3 is to BM2.

If you wanted to be more consistent in terms of expressions, you could rewrite the fourth order expansions as functions of f (as they are written here: http://permalink.lanl.gov/object/tr?what=info:lanl-repo/lareport/LA-UR-99-3356).

CaymanUnterborn · 2016-02-24T17:33:48Z

BM adopts a 3rd order P-V-K and a second or third order G, BM4 uses a 4th order P-V-K. I believe it was that reason that we split them.

CaymanUnterborn · 2016-02-24T17:34:13Z

And thanks for those equations, I'll rewrite. I like that formulation much better.

bobmyhill · 2016-02-24T17:36:04Z

Oh, yes, I remember. Hmmm, frustrating. I've opened a new issue (#234), because I still think BM should have a single home.

tjhei · 2016-02-26T16:11:49Z

Can we maybe get this merged as is and defer the cleanup to after the release?

CaymanUnterborn · 2016-02-26T19:27:50Z

Can you run me through exactly what I need to enter to rebase/squash correctly Timo?

tjhei · 2016-02-26T21:27:13Z

Looking through the history I can see that you merged from master at least once. Rebasing/Squashing is really awkward in this case. Luckily, there is an easy way to squash everything into a single commit:

I assume you call the official remote repo "upstream":

$ git remote -v
mygithub    git@github.com:tjhei/burnman.git (fetch)
mygithub    git@github.com:tjhei/burnman.git (push)
upstream    git@github.com:geodynamics/burnman.git (fetch)
upstream    git@github.com:geodynamics/burnman.git (push)

Make a backup:

git checkout Planets
git branch vinet_backup_before_merge

Get up to date:

git remote update
git merge upstream/master
# resolve conflict in misc/ref/table.py.out by throwing away your version:
git checkout misc/ref/table.py.out --theirs
git commit -a

Create another backup:

git branch vinet_backup_merged_before_squash

Rebase and squash in one step:

git reset --hard upstream/master
git merge --squash vinet_backup_merged_before_squash
git commit     # (make a nice commit message)

Test, make sure it looks okay.
Done, upload:
```
git push -f mygithub Planets
```

why the hell is your branch called "Planets"?

CaymanUnterborn · 2016-02-26T21:30:14Z

Because Planets are what I do Timo, the Earth is so passé these days :P

This is also where Planet builder was built so that's why it has that name.

I'll do this once I get back home this evening.

commit 2454445d3bfb8f08d4e97a02217b2dce82436c9d Merge: 74be206 cb7e671 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Feb 27 00:04:40 2016 -0500 Merge remote-tracking branch 'upstream/master' into Planets commit 74be206 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Thu Feb 25 16:13:43 2016 -0500 Cleaned up tests, more to do commit b47c22f Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Wed Feb 24 14:29:51 2016 -0500 Fixed default method in example. Reformatted BM4 to f notation commit cb8e698 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Wed Feb 24 11:08:25 2016 -0500 Fixed comments commit a83a921 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Mon Feb 22 19:43:16 2016 -0500 Added tests and benchmarks for Vinet and BM4 commit e151d90 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Fri Feb 12 12:21:40 2016 -0500 Added vinet to example_user_input commit 8e0a7df Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Thu Jan 7 15:04:30 2016 -0500 Cosmetic fixes commit 167e261 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 14:14:54 2016 -0500 Added K back in. commit 5c515f7 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 14:11:09 2016 -0500 Redundant Code removal commit dded24a Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 14:06:41 2016 -0500 Removed K, bracketed V commit be7f28b Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 13:59:56 2016 -0500 Cleanup commit cfcd96c Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 13:46:29 2016 -0500 Split BM into 4th order commit 2c96d8a Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 05:22:09 2016 -0500 Fixed decmials commit 3359fe0 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 05:18:47 2016 -0500 Removed unnecessary Fe mineral commit ec8c5d5 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 05:16:57 2016 -0500 Removing duplicate mineral commit ca412fa Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 04:48:48 2016 -0500 Cleaned up failed test commit bea8270 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 04:44:34 2016 -0500 Added a liquid Fe EOS for BM4 Calculations commit 7ba35ed Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 04:41:41 2016 -0500 Fixed decimals commit d579344 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 04:35:31 2016 -0500 Added New line Timo Requested commit b0af75f Merge: a8325da d5d3d03 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Tue Jan 5 04:24:34 2016 -0500 Merge remote-tracking branch 'geodynamics/master' into Planets commit a8325da Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sun Dec 13 14:09:58 2015 -0800 Fixed uppercase/lowercase issue commit 9de0593 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Dec 12 23:02:23 2015 -0800 Fixed typo causing test failure commit cf492ab Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Dec 12 22:27:12 2015 -0800 Lowercase V in vines and updated test table commit 200f3b6 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Dec 12 22:14:58 2015 -0800 Revert "Split Bm4 into it’s own EOS and reverted BM.py" This reverts commit 29a8b20. commit 018d5cb Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Dec 12 19:18:30 2015 -0800 Split Bm4 into it’s own EOS and reverted BM.py commit bb7f6d8 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Dec 12 18:15:27 2015 -0800 Fixed no new line issue commit fa7efd5 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Dec 12 18:06:08 2015 -0800 Fixed an import commit 3e830f3 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Sat Dec 12 18:02:53 2015 -0800 Included the 4th order Birch murnaghan EOS. Added 2 Epsilon Fe minerals in minerals.other and a liquid Fe EOS to use the 4th order BM EOS commit d274159 Author: Cayman Unterborn <kmanunterborn@gmail.com> Date: Thu Dec 10 18:54:27 2015 -0800 Added the Vinet EOS Fixed if to elif

CaymanUnterborn · 2016-02-27T05:12:29Z

Should be squashed/rebased.

tjhei · 2016-02-27T14:40:03Z

@bobmyhill @sannecottaar is this okay for the release as is? We can fix tests/design at some point.

I will fix the test before merging of course.

tjhei · 2016-03-01T17:36:27Z

@bobmyhill @sannecottaar ?

sannecottaar · 2016-03-01T17:47:51Z

I agree with Bob that BM should be organized differently, but otherwise I
think this looks good for now.
Thanks, Cayman.

On Tue, Mar 1, 2016 at 5:36 PM, Timo Heister notifications@github.com
wrote:

@bobmyhill https://github.com/bobmyhill @sannecottaar
https://github.com/sannecottaar ?

—
Reply to this email directly or view it on GitHub
#158 (comment).

bobmyhill · 2016-03-01T17:48:46Z

Oops, sorry, meant to comment and didn't. Yes, this is okay for the release. There's nothing wrong with the code; it just needs some tidying. I'm not a big fan of including the Anderson and Ahrens as a test case, as their EoS is an adiabatic one starting from the melting point at 100 kPa, not an isothermal one starting at 0 or 300 K. But if it recreates the figure in the paper (Figure B1), then it's ok.

Add Vinet and BM4

tjhei · 2016-03-15T17:33:37Z

I guess this can be closed. @lordkman are you going to reorganize things for after the release?

CaymanUnterborn · 2016-03-15T17:37:11Z

I will indeed. Already preparing some new things to add and I'll just combine that into it.

sannecottaar reviewed Dec 11, 2015
View reviewed changes

CaymanUnterborn force-pushed the Planets branch from d2b7fc2 to ea8fea9 Compare December 13, 2015 01:56

ian-r-rose reviewed Dec 13, 2015
View reviewed changes

CaymanUnterborn force-pushed the Planets branch from 29a8b20 to cf492ab Compare December 13, 2015 06:28

tjhei reviewed Dec 15, 2015
View reviewed changes

ian-r-rose reviewed Jan 5, 2016
View reviewed changes

ian-r-rose reviewed Jan 7, 2016
View reviewed changes

tjhei reviewed Jan 7, 2016
View reviewed changes

tjhei reviewed Feb 24, 2016
View reviewed changes

bobmyhill reviewed Feb 24, 2016
View reviewed changes

CaymanUnterborn force-pushed the Planets branch from 74be206 to 8dac81b Compare February 27, 2016 05:14

tjhei added a commit that referenced this pull request Mar 2, 2016

Merge PR #158 manually

54a37c1

Add Vinet and BM4

tjhei closed this Mar 15, 2016

Added the Vinet EOS #158

Added the Vinet EOS #158

Conversation

CaymanUnterborn commented Dec 11, 2015

Choose a reason for hiding this comment

tjhei commented Dec 11, 2015

Choose a reason for hiding this comment

tjhei commented Dec 13, 2015

Choose a reason for hiding this comment

sannecottaar commented Jan 5, 2016

CaymanUnterborn commented Jan 5, 2016

tjhei commented Jan 5, 2016

tjhei commented Jan 5, 2016

CaymanUnterborn commented Jan 5, 2016

tjhei commented Jan 5, 2016

tjhei commented Jan 5, 2016

CaymanUnterborn commented Jan 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CaymanUnterborn commented Jan 7, 2016

Choose a reason for hiding this comment

ian-r-rose commented Jan 7, 2016

tjhei commented Jan 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CaymanUnterborn commented Feb 16, 2016

bobmyhill commented Feb 17, 2016

CaymanUnterborn commented Feb 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjhei commented Feb 24, 2016

tjhei commented Feb 24, 2016

Choose a reason for hiding this comment

bobmyhill commented Feb 24, 2016

Choose a reason for hiding this comment

CaymanUnterborn commented Feb 24, 2016

bobmyhill commented Feb 24, 2016

CaymanUnterborn commented Feb 24, 2016

CaymanUnterborn commented Feb 24, 2016

bobmyhill commented Feb 24, 2016

tjhei commented Feb 26, 2016

CaymanUnterborn commented Feb 26, 2016

tjhei commented Feb 26, 2016

CaymanUnterborn commented Feb 26, 2016

CaymanUnterborn commented Feb 27, 2016

tjhei commented Feb 27, 2016

tjhei commented Mar 1, 2016

sannecottaar commented Mar 1, 2016

bobmyhill commented Mar 1, 2016

tjhei commented Mar 15, 2016

CaymanUnterborn commented Mar 15, 2016