simpeg icon indicating copy to clipboard operation
simpeg copied to clipboard

Add ElectricDipole

Open prisae opened this issue 1 year ago • 5 comments

Summary

Add an ElectricDipole to simpeg/electromagnetics/frequency_domain/sources.py similar to MagDipole.

Last piece for SimPEG(emg3d).

PR Checklist

  • [ ] If this is a work in progress PR, set as a Draft PR
  • [x] Linted my code according to the style guides.
  • [x] Added tests to verify changes to the code.
  • [x] Added necessary documentation to any new functions/classes following the expect style.
  • [x] Marked as ready for review (if this is was a draft PR), and converted to a Pull Request
  • [x] Tagged @simpeg/simpeg-developers when ready for review.

@simpeg/simpeg-developers - I think this is ready, feedback welcomed!

prisae avatar Aug 27 '24 15:08 prisae

Codecov Report

:x: Patch coverage is 60.00000% with 10 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 86.01%. Comparing base (1d73907) to head (b25c82b).

Files with missing lines Patch % Lines
tests/em/fdem/forward/test_FDEM_sources.py 40.00% 8 Missing and 1 partial :warning:
...impeg/electromagnetics/frequency_domain/sources.py 90.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1525      +/-   ##
==========================================
- Coverage   86.02%   86.01%   -0.02%     
==========================================
  Files         419      419              
  Lines       54795    54820      +25     
  Branches     5204     5208       +4     
==========================================
+ Hits        47137    47152      +15     
- Misses       6261     6270       +9     
- Partials     1397     1398       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Aug 27 '24 15:08 codecov[bot]

@dccowan, I think this PR is ready to be merged, but I'd like to hear your thoughts on it.

santisoler avatar Aug 27 '24 20:08 santisoler

I rewrote the part of SimPEG(emg3d) to use the RawVec-versions, which is more general for any possible source field. So I do not need this PR any longer.

However, it might still be valuable to have it. Indeed, it is just a special case of LineCurrent, so I changed it to be a subclass of LineCurrent. But because it is only a single point, it requires the orientation. Not sure if my implementation as such works.

Let me know if I shall close it, or if it is worth bringing it in.

prisae avatar Sep 06 '24 15:09 prisae

Hi @prisae! During last week's SimPEG meeting, @lheagy mentioned that the ElectricDipole class won't work with SimPEG simulation as it is because it lacks the s_m and s_e methods. I'm not sure how useful would be to have this class in SimPEG since you don't need it anymore. Maybe @lheagy and @dccowan can comment on that.

santisoler avatar Sep 09 '24 16:09 santisoler

Since the latest push it does have the s_e and s_m methods, as it is now just a special case of LineCurrent. But I am not entirely sure if it works this way with orientation. However, in the corresponding MagDipole this seems to work. Anyway, I am fine either way!

prisae avatar Sep 09 '24 17:09 prisae