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

Graphite from Shirasu 1993 #286

Merged
merged 3 commits into from
Jun 6, 2024
Merged

Graphite from Shirasu 1993 #286

merged 3 commits into from
Jun 6, 2024

Conversation

RemDelaporteMathurin
Copy link
Owner

Fixes #284

Copy link

codecov bot commented May 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.75%. Comparing base (404e9e4) to head (839f131).
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #286   +/-   ##
=======================================
  Coverage   98.75%   98.75%           
=======================================
  Files          76       76           
  Lines        1843     1853   +10     
=======================================
+ Hits         1820     1830   +10     
  Misses         23       23           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@FuerstT FuerstT left a comment

Choose a reason for hiding this comment

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

Need to correct your H/M unit conversion by adding Avogadro's number. E_S for each property is incorrect. It should be the value of B with units of K^-1. See Fig 2. Looks like graphite is an exothermic occluder and solubility decreases with temperature.


carbon_atomic_mass = 12.011 * u.g / u.mol

graphite_atomic_density = graphite_density / carbon_atomic_mass
Copy link
Contributor

Choose a reason for hiding this comment

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

You need Avogadro's number in there to convert to atomic density.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is handled by pint automatically. Pint knows that 1 mol = 6.022e23 particles


shirasu_solubility_IG_110U = Solubility(
S_0=np.exp(-14.5) * graphite_atomic_density * u.Pa**-0.5,
E_S=-18.2 * u.kJ / u.mol,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the incorrect value for activation energy. It should be the B value in Table 1 with units of K^-1.

Copy link
Owner Author

Choose a reason for hiding this comment

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

The paper says that the enthalpy of solution ($\Delta H$) is derived from $B$ though, is it not the same thing?

Copy link
Owner Author

@RemDelaporteMathurin RemDelaporteMathurin Jun 6, 2024

Choose a reason for hiding this comment

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

I confirm that this is correct:

B = 2190 * u.K
E = -htm.k_B * B

print(E.to(u.kJ / u.mol))

produces

-18.207968941253718 kilojoule / mole

Plus, $B$ has units of K not K^-1 since $\ln{K} = A + B/T$

graphite_atomic_density = graphite_density / carbon_atomic_mass

shirasu_solubility_IG_110U = Solubility(
S_0=np.exp(-14.5) * graphite_atomic_density * u.Pa**-0.5,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm dumb and does Pint bring units over in the calculation? Ideally you want units of H atoms * m^-3 * Pa^0.5.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Everything is converted to H atoms m^-3 Pa^-0.5 as long as you give it something with the same dimension.
For example, if you give mol cm^-3 Torr-0.5 then HTM (pint) will figure it out

If by mistake you don't give it the right dimension, then an error will be raised.

@RemDelaporteMathurin
Copy link
Owner Author

See Fig 2. Looks like graphite is an exothermic occluder and solubility decreases with temperature

Yes, this is why the activation energies are negative

Copy link
Contributor

@FuerstT FuerstT left a comment

Choose a reason for hiding this comment

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

Thanks for the lessons! Looks good then.

@RemDelaporteMathurin RemDelaporteMathurin merged commit ff4de17 into main Jun 6, 2024
9 checks passed
@RemDelaporteMathurin RemDelaporteMathurin deleted the graphite--shirasu branch June 6, 2024 15:33
Copy link
Contributor

@FuerstT FuerstT left a comment

Choose a reason for hiding this comment

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

Looks good then! Thanks for the lesson.

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.

Property suggestion: graphite solubility
2 participants