Skip to content

Add linearity tests for forward models - #698

Open
manduinca wants to merge 1 commit into
fatiando:mainfrom
manduinca:add-linearity-tests
Open

Add linearity tests for forward models#698
manduinca wants to merge 1 commit into
fatiando:mainfrom
manduinca:add-linearity-tests

Conversation

@manduinca

Copy link
Copy Markdown

Fixes #293

Adds test/test_linearity.py with linearity tests for each geometry (points, prisms, tesseroids), parametrized by field, following the discussion in the issue: the field of a set of sources is compared against the sum of the fields of each source computed separately.

Includes both gravity and magnetic forward models: point_gravity (Cartesian and spherical coordinates), dipole_magnetic, prism_gravity, prism_magnetic and tesseroid_gravity, using mixed-sign masses/densities/magnetizations.

The newer ellipsoid_gravity/ellipsoid_magnetic forwards could get the same treatment in a follow-up if you'd like.

Test that the field generated by a set of sources equals the sum of the
fields of each source computed separately. Covers point_gravity (Cartesian
and spherical), dipole_magnetic, prism_gravity, prism_magnetic and
tesseroid_gravity, parametrized by field.

@santisoler santisoler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @manduinca! Sorry for the delayed review. I was busy at a conference and catching up with stuff.

This looks really nice, thanks again for taking the effort and work on it! I left some minor suggestions below. Nothing major, just to make a few portions of the tests more Pythonic. Please, let me know what do you think, and feel free to apply and modify the suggestions as you like.

Comment thread test/test_linearity.py
Comment on lines +237 to +241
sample_prisms[i : i + 1],
sample_densities[i : i + 1],
field,
)
for i in range(sample_densities.size)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor suggestion here: we don't need to index the arrays, we can iterate over them with a zip function:

Suggested change
sample_prisms[i : i + 1],
sample_densities[i : i + 1],
field,
)
for i in range(sample_densities.size)
prism,
density,
field,
)
for prism, density in zip(sample_prisms, sample_densities, strict=True)

Comment thread test/test_linearity.py
Comment on lines +366 to +370
sample_tesseroids[i : i + 1],
sample_densities[i : i + 1],
field=field,
)
for i in range(sample_densities.size)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Similarly here:

Suggested change
sample_tesseroids[i : i + 1],
sample_densities[i : i + 1],
field=field,
)
for i in range(sample_densities.size)
tesseroid,
density,
field=field,
)
for tesseroid, density in zip(sample_tesseroids, sample_densities, strict=True)

Comment thread test/test_linearity.py
Comment on lines +87 to +88
tuple(p[i : i + 1] for p in sample_points),
sample_masses[i : i + 1],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would suggest indexing the arrays directly, so we pass only a float for each coordinate and for the mass. This way we are also testing (although we are probably already testing that in other places) that passing just a float as coordinates and single float for mass also works.

Suggested change
tuple(p[i : i + 1] for p in sample_points),
sample_masses[i : i + 1],
tuple(p[i] for p in sample_points),
sample_masses[i],

Comment thread test/test_linearity.py
Comment on lines +115 to +116
tuple(p[i : i + 1] for p in points),
sample_masses[i : i + 1],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Similarly here:

Suggested change
tuple(p[i : i + 1] for p in points),
sample_masses[i : i + 1],
tuple(p[i] for p in points),
sample_masses[i],

Comment thread test/test_linearity.py
Comment on lines +178 to +179
tuple(d[i : i + 1] for d in sample_dipoles),
tuple(m[i : i + 1] for m in sample_magnetic_moments),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And here:

Suggested change
tuple(d[i : i + 1] for d in sample_dipoles),
tuple(m[i : i + 1] for m in sample_magnetic_moments),
tuple(d[i] for d in sample_dipoles),
tuple(m[i] for m in sample_magnetic_moments),

Comment thread test/test_linearity.py
Comment on lines +301 to +302
sample_prisms[i : i + 1],
tuple(m[i : i + 1] for m in sample_magnetizations),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
sample_prisms[i : i + 1],
tuple(m[i : i + 1] for m in sample_magnetizations),
sample_prisms[i],
tuple(m[i] for m in sample_magnetizations),

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.

Add linearity tests for forward models

2 participants