Patch mass_density() & update Alfven_speed() #957

rocco8773 · 2020-12-04T04:27:38Z

This PR focuses on the mass_density() and Alfven_speed() functions. Alfven_speed() is touched here because it's currently the only functionality that utilizes mass_density()

mass_density()

The original mass density function was a little un-intuitive since it would actually calculate the total mass density (ions + electrons) from a number density opposed to just the ion mass density or electron mass density. The new mass density function only calculates the mass density for the particle specified.

How it now works...

>>> # if a mass density is passed in then it is returned
>>> mass_density(1 * u.kg * u.m ** -3, "He")
<Quantity 1. kg/m3>

>>> # number density is assumed to be associated with the particle of interest
>>> mass_density(1e18 * u.m ** -3, "He")
<Quantity 6.64647699e-09 kg / m3>

>>> # z_ratio keyword can be used if the number density is associated with a different species
>>> ne = 1e18 * u.m ** -3
>>> par = Particle("He 2+")
>>> mass_density(ne, par, z_ratio=1/par.integer_charge)
<Quantity 3.32232756e-09 kg / m3>

Alfven_speed()

Original functionality does not change. I just updated the code to handle the new mass_density() function and implemented handling of Particle(). This Particle handling could be dropped/modified once the particle_like annotation is implemented (see issue #912). I also restructured the tests, since that was marked as a TODO.

…ticle, and not "total" mass density that includes the electron component

…sity type

…; add conditioning for kwargs "ion" and "z_mean"

…ng pytest.mark.parametrize

namurphy

A quick partial review before a meeting is about to start. I'll try to take another look soon, though please remind me if I forget. Thank you for working on this!

plasmapy/formulary/parameters.py

Co-authored-by: Nick Murphy <[email protected]>

namurphy

Thank you for working on this! I have a few typos plus a few more in-depth comments.

In the longer term, I'm wondering about how to deal with things like z_mean. Having z_mean as a keyword along with ion has the potential for a contradiction, i.e. a case where the ion has one charge and z_mean has a different charge...at least if we assume quasineutrality. We could use CustomParticle for things like this, since this is the sort of situation it was intended for. Since we now have CustomParticle, we could drop z_mean as an argument. Hm...

In the long run, perhaps Particle could take charges that are non-integers, i.e. Particle("He-4 1.5+") for the case with z_mean=1.5...and we could get back a CustomParticle. Then Particle could end up being an implementation of the factory design pattern, which maybe it should be. But that's not something for 2020.

The suggestions about using @particle_input might be beyond the scope of this PR, so they don't have to be implemented here.

plasmapy/formulary/parameters.py

namurphy · 2020-12-08T00:10:24Z

plasmapy/formulary/parameters.py

+        if not isinstance(ion, Particle):
+            try:
+                ion = Particle(ion)
+            except TypeError:
+                raise TypeError(
+                    f"If passing a number density, you must pass a plasmapy Particle "
+                    f"(not type {type(ion)}) to calculate the mass density!"
+                )


Perhaps use @particle_input for this function instead?

No, for the same reason stated in https://github.com/PlasmaPy/PlasmaPy/pull/957/files#r537960088.

namurphy · 2020-12-08T00:14:00Z

plasmapy/formulary/parameters.py

+    if not isinstance(particle, Particle):
+        try:
+            particle = Particle(particle)
+        except TypeError:
+            raise TypeError(
+                f"If passing a number density, you must pass a plasmapy Particle "
+                f"(not type {type(particle)}) to calculate the mass density!"


Perhaps use @particle_input to get this functionality instead?

We ca not do that at this point because the density annotation is a list. That is what was causing the error that made me do this patch. When I finally finish the update to particle_input that also adds particle_like, then we can replace this code block with @particle_input.

Ah! I hereby withdraw my suggestion.

namurphy · 2020-12-08T01:05:00Z

plasmapy/formulary/parameters.py

+    ion : `~plasmapy.particles.Particle`, optional
+        Representation of the ion species (e.g., `'p'` for protons, `'D+'` for
+        deuterium, `'He-4 +1'` for singly ionized helium-4, etc.). If no charge
+        state information is provided, then the ions are assumed to be singly
+        ionionized. If the density is an ion number density, then this paramter
+        is required in order to convert to mass density.
+
+    z_mean : `~numbers.Real`, optional
+        The average ionization state (arithmetic mean) of the ``ion`` composing
+        the plasma.  This is used in calculating the mass density
+        :math:`\rho = n_i (m_i + Z_{mean} m_e)`.  ``z_mean`` is ignored if
+        ``density`` is passed as a mass density.


One use case I'm wondering about is if ion is provided (e.g., He-4 2+) and z_mean is provided as 1. In this case there's a contradiction between ion and z_mean...at least if we want to assume quasineutrality, which is usually the case. (If I remember correctly, the derivation for the Alfvén speed implicitly assumes quasineutrality.) And if z_mean is provided, then ion would be an element instead so the name ion wouldn't be appropriate. I'm wondering about using the machinery of CustomParticle for situations like this, which probably won't arise particularly frequently. Hm...

Yes, the derivation of the Alfvén speed assumes quasi-neutrality. Now, if someone where to pass ion="He-4 +2" and z_mean=1.2, then the function will used the mass of He-4 +2 to calculate the ion mass density and z_mean=1.2 to calculate the electron mass density. So, z_mean will take precedence over any charge value indicated by ion.

As I mentioned in the element chat, I'm not sure having z_mean as a Particle attribute is the right approach because z_mean is really a plasma attribute.

plasmapy/formulary/parameters.py

Co-authored-by: Nick Murphy <[email protected]>

rocco8773 · 2020-12-08T01:40:34Z

@namurphy Thanks for the review! My code is always littered with spelling mistakes. I swear I actively look for them before requesting a review. 🤦‍♂️🤦‍♂️🤣🤣

StanczakDominik

LGTM, but seriously, please do fix your spellcheck settings, because the more attention reviewers spend fixing typos, the less there is to spare for actual bugs etc! ;)

plasmapy/formulary/parameters.py

StanczakDominik · 2020-12-08T20:36:00Z

plasmapy/formulary/parameters.py

+        \\rho = \\left| \\frac{Z_{s}}{Z_{particle}} \\right| n_{s} m_{particle}
+              = | Z_{ratio} | n_{s} m_{particle}


I get how beautifully this renders, but I do a double take every time I look at the source... 😅

plasmapy/formulary/parameters.py

StanczakDominik · 2020-12-08T20:49:53Z

plasmapy/formulary/parameters.py

+        The average ionization state (arithmetic mean) of the ``ion`` composing
+        the plasma.  This is used in calculating the mass density
+        :math:`\rho = n_i (m_i + Z_{mean} m_e)`.  ``z_mean`` is ignored if
+        ``density`` is passed as a mass density.


Note for the future: that's going to get interesting with impurities...

Co-authored-by: Dominik Stańczak <[email protected]>

…rovided by `ion`

rocco8773 · 2020-12-08T22:09:17Z

@StanczakDominik Thanks for pointing out the typos. For some reason my IDE does not even mark them.

I also fix a few more typos I noticed while fixing your suggestions.

rocco8773 added 14 commits November 21, 2020 19:19

rework mass_density so it calculates mass density associated with par…

e34ff00

…ticle, and not "total" mass density that includes the electron component

remove unused imports

2d9d919

add str to typing annotations for particle

a87d6f4

expand docstring tests

e336404

rewrite tests to cover modified mass_density

6477d08

remove unused imports

94c3d81

minor fixes to mass_density

38eaf6c

expand mass_density tests to include negative densities and not a den…

6de8f52

…sity type

Merge branch 'master_from_plasmapy' into patch_mass_density

4d53d40

add annotation for z_mean

67b8999

update Alfven_speed to account for updated mass_density functionality…

062ed29

…; add conditioning for kwargs "ion" and "z_mean"

update Alfven_speed docstring; update doctests

9a90b4f

rewrite Alfven_speed tests incorporating in to test class and utilizi…

6dcdbc1

…ng pytest.mark.parametrize

appease grumpy black

eaf087a

rocco8773 added the plasmapy.formulary Related to the plasmapy.formulary subpackage label Dec 4, 2020

rocco8773 self-assigned this Dec 4, 2020

rocco8773 requested review from namurphy and StanczakDominik December 4, 2020 04:27

rocco8773 added 2 commits December 3, 2020 20:47

black (v19.10b0) fixes

9d7c1a1

add changelogs

497ebaa

namurphy requested changes Dec 4, 2020

View reviewed changes

plasmapy/formulary/parameters.py Outdated Show resolved Hide resolved

plasmapy/formulary/parameters.py Outdated Show resolved Hide resolved

rocco8773 and others added 2 commits December 4, 2020 09:52

Apply suggestions from code review

a04978b

Co-authored-by: Nick Murphy <[email protected]>

remove extra ~ at the end

26b1bff

namurphy requested changes Dec 8, 2020

View reviewed changes

fix a bunch of typos

455c298

Co-authored-by: Nick Murphy <[email protected]>

minor text fixes :D

01c81e2

StanczakDominik approved these changes Dec 8, 2020

View reviewed changes

rocco8773 and others added 5 commits December 8, 2020 13:47

typo denisty -> density

1c6f1a4

fix grammar

976ed00

typo plamsapy -> plasmapy

196d685

fix a bunch of typos

0532b1c

Co-authored-by: Dominik Stańczak <[email protected]>

in docstring specifically stat z_mean overrides charge state info p…

cef1a57

…rovided by `ion`

StanczakDominik merged commit 031a8f3 into PlasmaPy:master Dec 8, 2020

rocco8773 deleted the patch_mass_density branch December 10, 2020 18:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Patch mass_density() & update Alfven_speed() #957

Patch mass_density() & update Alfven_speed() #957

rocco8773 commented Dec 4, 2020

namurphy left a comment

namurphy left a comment

namurphy Dec 8, 2020

rocco8773 Dec 8, 2020

namurphy Dec 8, 2020

rocco8773 Dec 8, 2020

namurphy Dec 8, 2020

namurphy Dec 8, 2020

rocco8773 Dec 8, 2020

rocco8773 commented Dec 8, 2020

StanczakDominik left a comment

StanczakDominik Dec 8, 2020

StanczakDominik Dec 8, 2020

rocco8773 commented Dec 8, 2020

		\\rho = \\left\| \\frac{Z_{s}}{Z_{particle}} \\right\| n_{s} m_{particle}
		= \| Z_{ratio} \| n_{s} m_{particle}

Patch mass_density() & update Alfven_speed() #957

Patch mass_density() & update Alfven_speed() #957

Conversation

rocco8773 commented Dec 4, 2020

namurphy left a comment

Choose a reason for hiding this comment

namurphy left a 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

Choose a reason for hiding this comment

rocco8773 commented Dec 8, 2020

StanczakDominik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rocco8773 commented Dec 8, 2020