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

Add T computation for Ti and Zr in Rutile and zircon, from Ferry and Watson, 2007 #67

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

Iddingsite
Copy link
Contributor

@Iddingsite Iddingsite commented Jun 14, 2024

Hi,

I needed this so I've implemented the functions that gives temperature from Zr and Ti in zircon and rutile (only the ones giving Ti and Zr were implemented before).
Tests were also added.

Just a more general question, is there a reason why the package Measurements.jl is not used in this package? I think it would be a nice feature to have uncertainty propagated from this kind of functions.

I was thinking about something like that to not break anything:

function Ferry_Ti_in_zirconT(Ti::Number, aSiO2::Number, aTiO2::Number; uncertainty=false)
    if uncertainty
        1 / ((5.711±0.072) - log10(aSiO2) + log10(aTiO2) - log10(Ti)) * (4800.0±86) .- 273.15
    else
        1 / ((5.711) - log10(aSiO2) + log10(aTiO2) - log10(Ti)) * (4800.0) .- 273.15
    end
end

What do you think?

Also, I have local tests failure unrelated to this PR due to the test of Perplex on line 245 of testGeochemistry.jl (I am on Mac). That would probably be fixed by using the Perple_X_jll at some point.

Cheers,

@brenhinkeller
Copy link
Owner

Thanks! Seems like a good add!

I'd be happy to have a Measurements.jl dependency added, and dispatching on that seems like a nice way to do it

The Mac Perplex test failures are probably just because the package currently only knows how to automatically install perplex on linux, so one has to manually install perplex in the right location otherwise. That's a bit of a pain but could be solved if we manage to switch to using perplex_jll (#28)

@brenhinkeller brenhinkeller merged commit 9f221e3 into brenhinkeller:main Jun 14, 2024
3 of 4 checks passed
@Iddingsite
Copy link
Contributor Author

Iddingsite commented Jun 14, 2024

Ok, I will give it a shot next week then. Have a good wk-end!

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.

2 participants