-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Derby-Olbert Ideal Solenoid #92
base: master
Are you sure you want to change the base?
Conversation
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.
Looks good. Stream plot is really nice!
|
||
""" | ||
|
||
test_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.
I like pytest.mark.parameterize
for test cases like this
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.
Expanding on that a bit:
Right now, your test function test_cel_vs_c_full
is run once with all inputs. If one should fail, the rest aren't even tested. Parametrizing your inputs means your test means that the test function will be run independently, once for each set of inputs you provide. The only downside is that the syntax can be a bit confusing. You have a few options, the first being my general preference:
- Positional arguments
@pytest.mark.parametrize(
("kc", "p", "c", "s", "mma"),
[
pytest.param(0.5, 1.0, 1.0, 0.5, 1.5262092342121871, id="kc0.5"),
pytest.param(0.0, 1.0, 1.0, 0.0, 1.0, id="edge-case"),
],
)
def test_cel_vs_c_full(kc: float, p: float, c: float, s: float, mma: float):
print(kc, p, c, s, mma)
which looks like:
$ pytest -v test_a.py
...
test_a.py::test_cel_vs_c_full[kc0.5] PASSED [ 50%]
test_a.py::test_cel_vs_c_full[edge-case] PASSED [100%]
- Dictionary of params, similar to what you have
@pytest.mark.parametrize(
("params",),
[
pytest.param(
{"kc": 0.5, "p": 1.0, "c": 1.0, "s": 0.5, "mma": 1.5262092342121871},
id="kc0.5",
),
pytest.param(
{"kc": 0.0, "p": 1.0, "c": 1.0, "s": 0.0, "mma": 1.0}, id="edge-case"
),
],
)
def test_cel_vs_c_full_params(params: dict[str, float]):
print(params)
- Reusing your dictionary in a lazy approach:
test_cases = [
{"kc": 0.5, "p": 1.0, "c": 1.0, "s": 0.5, "mma": 1.5262092342121871},
{"kc": 0.8, "p": 0.9, "c": 1.2, "s": -0.3, "mma": 0.7192092915373303},
{"kc": 0.3, "p": 0.5, "c": 2.0, "s": 0.7, "mma": 4.371297871647941},
{"kc": 0.0, "p": 1.0, "c": 1.0, "s": 0.0, "mma": 1.0}, # Edge case
]
@pytest.mark.parametrize(("params",), [[case] for case in test_cases])
def test_cel_vs_c_full_params1(params: dict[str, float]):
print(params)
Another couple things which aren't useful in this exact scenario:
- Multiple specifications of the
parametrize
decorator will give you all permutations of the input values. It's pretty powerful! - You can actually define a decorator as
use_my_params = pytest.mark.parametrize(...)
and reuse it on multiple functions as@use_my_params
.
f" Difference: {difference}\n" | ||
) | ||
|
||
assert np.isclose(cel_result, c_full_result) |
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.
Numpy has some good testing functions I use in this application.
On error, it gives a more informative readout than "assert np.False_" from pytest.
if nI is None and B0 is not None: | ||
nI = B0 * np.hypot(radius, L / 2) / (mu_0 * L / 2) | ||
elif nI is None and B0 is None: | ||
B0 = 1 |
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.
nI needs to be set in this case?
np.ndarray | ||
Normalized magnetic field values at the specified z positions. | ||
""" | ||
a = radius |
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.
Should this just call Bz_on_axis
?
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 plots do look really nice!
|
||
""" | ||
|
||
test_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.
Expanding on that a bit:
Right now, your test function test_cel_vs_c_full
is run once with all inputs. If one should fail, the rest aren't even tested. Parametrizing your inputs means your test means that the test function will be run independently, once for each set of inputs you provide. The only downside is that the syntax can be a bit confusing. You have a few options, the first being my general preference:
- Positional arguments
@pytest.mark.parametrize(
("kc", "p", "c", "s", "mma"),
[
pytest.param(0.5, 1.0, 1.0, 0.5, 1.5262092342121871, id="kc0.5"),
pytest.param(0.0, 1.0, 1.0, 0.0, 1.0, id="edge-case"),
],
)
def test_cel_vs_c_full(kc: float, p: float, c: float, s: float, mma: float):
print(kc, p, c, s, mma)
which looks like:
$ pytest -v test_a.py
...
test_a.py::test_cel_vs_c_full[kc0.5] PASSED [ 50%]
test_a.py::test_cel_vs_c_full[edge-case] PASSED [100%]
- Dictionary of params, similar to what you have
@pytest.mark.parametrize(
("params",),
[
pytest.param(
{"kc": 0.5, "p": 1.0, "c": 1.0, "s": 0.5, "mma": 1.5262092342121871},
id="kc0.5",
),
pytest.param(
{"kc": 0.0, "p": 1.0, "c": 1.0, "s": 0.0, "mma": 1.0}, id="edge-case"
),
],
)
def test_cel_vs_c_full_params(params: dict[str, float]):
print(params)
- Reusing your dictionary in a lazy approach:
test_cases = [
{"kc": 0.5, "p": 1.0, "c": 1.0, "s": 0.5, "mma": 1.5262092342121871},
{"kc": 0.8, "p": 0.9, "c": 1.2, "s": -0.3, "mma": 0.7192092915373303},
{"kc": 0.3, "p": 0.5, "c": 2.0, "s": 0.7, "mma": 4.371297871647941},
{"kc": 0.0, "p": 1.0, "c": 1.0, "s": 0.0, "mma": 1.0}, # Edge case
]
@pytest.mark.parametrize(("params",), [[case] for case in test_cases])
def test_cel_vs_c_full_params1(params: dict[str, float]):
print(params)
Another couple things which aren't useful in this exact scenario:
- Multiple specifications of the
parametrize
decorator will give you all permutations of the input values. It's pretty powerful! - You can actually define a decorator as
use_my_params = pytest.mark.parametrize(...)
and reuse it on multiple functions as@use_my_params
.
def make_solenoid_fieldmesh( | ||
*, | ||
rmin: float = 0, | ||
rmax: float = None, |
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.
These aren't valid default values for the annotation float
If you want these to be optional with None
, it has to read either:
- Python 3.10+ compatible syntax:
float | None = None
rmax: float = None, | |
rmax: float | None = None, |
- Python 3.9+ compatible syntax:
Optional[float] = None
(this requires afrom typing import Optional
at the top)
*, | ||
rmin: float = 0, | ||
rmax: float = None, | ||
zmin: float = None, |
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.
I think rmax
, zmax
, and radius
are actually all required to be not None
based on the code
This adds modeling of an ideal solenoid from the formulas in:
Derby, N., & Olbert, S. (2010). Cylindrical magnets and ideal solenoids.
American Journal of Physics, 78(3), 229–235. https://doi.org/10.1119/1.3256157
This adds:
pmd_beamphysics.fields.solenoid
with:make_solenoid_fieldmesh
to make a newFieldMesh
objectfit_ideal_solenoid
to fit the length and radius of on-axis solenoid data to the ideal modelExamples: