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

docs: fix typos #304

Merged
merged 4 commits into from
Oct 20, 2024
Merged

docs: fix typos #304

merged 4 commits into from
Oct 20, 2024

Conversation

Mo-Gul
Copy link
Contributor

@Mo-Gul Mo-Gul commented Oct 19, 2024

and improve some formatting/style.

@Mo-Gul
Copy link
Contributor Author

Mo-Gul commented Oct 19, 2024

Other things can be harmonized as well, e.g. the use of d in \int ... dx. There is one instance of an upright d

``A_\mathrm{ideal} =- \int p\,\mathrm{d}V =- Nk_\mathrm{B}T\ln{V}+c(T,N)``

The other (five) instances (just) use the normal math d (italics).

@longemen3000
Copy link
Member

Thanks!
on the use of italics vs upright d, feel free to add those changes here or in another PR. I'm going to merge this tomorrow.

@Mo-Gul
Copy link
Contributor Author

Mo-Gul commented Oct 20, 2024

What I have also noticed is that you (sometimes) write "personal names" in small letters like "antonine"

For saturation solvers ([`saturation_pressure`](@ref),[`saturation_temperature`](@ref)), You can dispatch on a different saturation method. let's create one, that just evaluates antoine coefficients:

or "helmholtz"

Almost all models in Clapeyron based on helmholtz free energy have at least one of the following functions defined:

Is that on purpose?

@longemen3000
Copy link
Member

Is that on purpose?
No haha, feel free to correct those docs.

@Mo-Gul
Copy link
Contributor Author

Mo-Gul commented Oct 20, 2024

Ok, one more thing I have noticed. Have a look at the items in https://clapeyronthermo.github.io/Clapeyron.jl/dev/user_guide/custom_methods/#Definitions

which is the output of

- `[volume](@ref)(model, p, T, z)`: Obtains the volume of a system at a given temperature, pressure and composition. If the phase is unknown, it will find the vapour and liquid roots and return the one that minimises the Gibbs free energy. This function is called by all of our bulk property methods.
- `[saturation_pressure](@ref)(model, T)`: Obtains the saturation pressures and volumes for a pure species.
- `[crit_pure](@ref)(model)`: Obtains the critical point for a pure species.

I am guessing that the starts of the lines should look something similar to

[`volume(model, p, T, z)`](@ref)

right? But I don't have a clue if this gives the expected result.

@longemen3000
Copy link
Member

I am guessing that the starts of the lines should look something similar to

i just checked the docs for Documenter.jl, and the correct syntax for those cases is actually:

[volume(model,p,T,z](@ref Clapeyron.volume)

https://documenter.juliadocs.org/stable/man/syntax/#Named-@refs

@Mo-Gul
Copy link
Contributor Author

Mo-Gul commented Oct 20, 2024

I'd say this is something for another PR which I leave up for you, just in case this doesn't work (directly). Are you fine with that?

@longemen3000
Copy link
Member

yeah, i will test if that works later. in the meantime, i think this PR is ready to merge.

@pw0908
Copy link
Member

pw0908 commented Oct 20, 2024

Mind if I take a look before merging?

@pw0908
Copy link
Member

pw0908 commented Oct 20, 2024

@Mo-Gul thanks a lot for all of this! I looked over your modifications and have no issues with any of them.

If it is of interest to you, we are modifying the docs over on a separate branch. Progress has been slow, but we'd be grateful for any feedback you might provide!

@Mo-Gul
Copy link
Contributor Author

Mo-Gul commented Oct 20, 2024

Sure, but since I just forked the master branch it would be nice to first merge this PR. After that I'll refork and have a look.

Shall I start a discussion for general things about the docs or what would be a proper place for that?

@pw0908 pw0908 merged commit 68d8625 into ClapeyronThermo:master Oct 20, 2024
@pw0908
Copy link
Member

pw0908 commented Oct 20, 2024

Done!

Discussion would probably be the best place for it.

@Mo-Gul Mo-Gul mentioned this pull request Oct 23, 2024
19 tasks
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