-
Notifications
You must be signed in to change notification settings - Fork 87
Refactor code to have a single Ellipsoid class
#616
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
Draft
santisoler
wants to merge
30
commits into
fatiando:main
Choose a base branch
from
santisoler:single-ellipsoid-class
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Start implementing gravity field with a permutation matrix to sort out the semiaxes.
Update the elliptical integrals for oblates so they assume that `a == b > c`. Simplify the permutation matrix since now we don't need to take care of oblates as a special case.
Remove quite a lot of tests since we now have a single ellipsoid class.
It makes it more explicit about what's happening with the semiaxes. The permutation matrix should match that sorting, but we can cover that in tests.
Update the magnetic code to use the single `Ellipsoid` class, make use of the permutation matrix, and update the internal demagnetization tensor equations for the oblate ellipsoid defined as `a == b > c`.
Do this to differentiate it from just the rotation matrix, since it also includes the permutation one.
Member
Author
|
TODO:
|
The permutation matrix used wasn't right: we need to apply a proper rotation matrix (with 90 degrees) to ensure that the local coordinate system is a right-handed one.
Test ellipsoids defined with semiaxes passed in arbitrary orders against equivalent ellipsoids defined with ``a >= b >= c``, but with necessary rotations.
After changing the definition of the oblate to `a == b > c`, we need to change the condition for oblates in such function.
Fix the numerical instabilities of gravity fields produced by triaxial ellipsoids that approximate a prolate and an oblate ellipsoid. Fallback to the prolate and oblate solutions when two of the three semiaxes are close enough to each other.
Fallback to the prolate and oblate solutions for the internal demagnetization tensor when triaxial has two semiaxes close enough to each other.
Use the ratio instead of a static delta, so we are sure we are comparing the two different codes and not just triggering the numerical instability fix that falls back to the spherical solutions.
Use the ratio instead of a static delta, so we are sure we are comparing the two different codes and not just triggering the numerical instability fix that falls back to the spherical solutions.
2a93219 to
d40d9fd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the ellipsoid code to have a single
Ellipsoidclass that can represent any type of ellipsoid. Ditch the specific classes for each ellipsoid type. Update the forward modelling code for gravity and magnetics. Make use of an extra rotation matrix that aligns the x, y, z local coordinate system to the ellipsoid's semiaxes in decreasing order, soa >= b >= c. Modify the analytic solutions for oblate ellipsoids in such way that they are defined bya == b > c(instead ofa < b = cas Takahashi et al. and Clark et al. do). Make rotation angles andcenteras optional arguments for theEllipsoidclass. Fix numerical instabilities of triaxial ellipsoids when they approximate prolate or oblate ellipsoids (when two semiaxes lengths are close enough to each other). Update the tests and add a few more for the new bits of code.Relevant issues/PRs:
Part of #594