-
-
Notifications
You must be signed in to change notification settings - Fork 46.5k
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
Adding avg and mps speed formulae for ideal gases #10229
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper
commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper review
to trigger the checks for only added pull request files@algorithms-keeper review-all
to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
maths/amicable_pair.py
Outdated
to find the sum of the proper divisors of a number. | ||
""" | ||
|
||
def sum_of_divisors(n: int) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide descriptive name for the parameter: n
maths/amicable_pair.py
Outdated
|
||
return sum | ||
|
||
def is_amicable_pair(a: int, b: int) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide descriptive name for the parameter: a
Please provide descriptive name for the parameter: b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper
commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper review
to trigger the checks for only added pull request files@algorithms-keeper review-all
to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
maths/amicable_pair.py
Outdated
""" | ||
|
||
|
||
def sum_of_divisors(n: int) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide descriptive name for the parameter: n
maths/amicable_pair.py
Outdated
return sum | ||
|
||
|
||
def is_amicable_pair(a: int, b: int) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide descriptive name for the parameter: a
Please provide descriptive name for the parameter: b
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
# necessary constants | ||
PI = 3.1415926535 # pi | ||
R = 8.3144626181 # gas constant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we import these constants from existing libraries rather than hard-coding them?
else: | ||
return (8 * R * temperature / (PI * molar_mass)) ** 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else: | |
return (8 * R * temperature / (PI * molar_mass)) ** 0.5 | |
return (8 * R * temperature / (PI * molar_mass)) ** 0.5 |
else: | ||
return (2 * R * temperature / molar_mass) ** 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else: | |
return (2 * R * temperature / molar_mass) ** 0.5 | |
return (2 * R * temperature / molar_mass) ** 0.5 |
if temperature < 0: | ||
raise Exception("Absolute temperature cannot be less than 0 K") | ||
if molar_mass <= 0: | ||
raise Exception("Molar mass should be greater than 0 kg/mol") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of completeness, could you add doctests that test these cases?
if temperature < 0: | ||
raise Exception("Absolute temperature cannot be less than 0 K") | ||
if molar_mass <= 0: | ||
raise Exception("Molar mass should be greater than 0 kg/mol") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with these cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename the file to speeds_of_gas_molecules.py
to keep it shorter
Incorporated all the suggestions in the file. Please check @tianyizheng02. Thank you for the detailed review. |
physics/speeds_of_gas_molecules.py
Outdated
--------------------- | ||
| vavg = √8RT/πM | | ||
--------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--------------------- | |
| vavg = √8RT/πM | | |
--------------------- | |
--------------------- | |
| vavg = √(8RT/πM) | | |
--------------------- |
For clarity
physics/speeds_of_gas_molecules.py
Outdated
--------------------- | ||
| vmp = √2RT/M | | ||
--------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--------------------- | |
| vmp = √2RT/M | | |
--------------------- | |
--------------------- | |
| vmp = √(2RT/M) | | |
--------------------- |
physics/speeds_of_gas_molecules.py
Outdated
--------------------- | ||
| vrms = √3RT/m | | ||
--------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--------------------- | |
| vrms = √3RT/m | | |
--------------------- | |
--------------------- | |
| vrms = √(3RT/M) | | |
--------------------- |
| vmp = √2RT/M | | ||
--------------------- | ||
|
||
The root-mean-square speed is another measure of the average speed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No function for the RMS speed?
physics/speeds_of_gas_molecules.py
Outdated
The root-mean-square, average and most probable speeds are derived from | ||
the Maxwell-Boltzmann distribution. The Maxwell-Boltzmann distribution is a | ||
probability distribution that describes the distribution of speeds for particles | ||
in a gas. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The root-mean-square, average and most probable speeds are derived from | |
the Maxwell-Boltzmann distribution. The Maxwell-Boltzmann distribution is a | |
probability distribution that describes the distribution of speeds for particles | |
in a gas. | |
The root-mean-square, average and most probable speeds of gas molecules are | |
derived from the Maxwell-Boltzmann distribution. The Maxwell-Boltzmann | |
distribution is a probability distribution that describes the distribution of | |
speeds of particles in an ideal gas. |
A few minor additions for clarity
Sure, adding the above documentation changes. I did not add the rms function as there is already a file that defines the function ie physics/rms_speed_of_molecule.py as that would be repeating the function. Should I add it here for a complete comparison of all three speeds? |
@tianyizheng02 please check if the current file is ok |
No need, I forgot that we already had a file for the RMS speed. However, it'd make more sense in my opinion to move the existing RMS speed code into your code file since they're all about gas molecule speeds—that could be work for a later PR. |
Thanks for your contribution! |
Glad to contribute! Thank you for taking time to review the file and giving great advice! |
* avg and mps speed formulae added * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * avg and mps speed formulae added * fixed_spacing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * spacing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * ws * added amicable numbers * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * added amicable numbers * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * spacing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * removed * changed name of file and added code improvements * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * issues fixed due to pi * requested changes added --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Describe your change:
Added functions to calculate the average and most probable speeds of molecules in a gas given the temperature and molar mass of the gas. Also added a few doctests for testing.
Checklist: