Skip to content
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

Extend BaseScr to that Pole and Dipole DC are simply minor extensions… #1007

Merged
merged 24 commits into from Apr 29, 2022

Conversation

ngodber
Copy link
Contributor

@ngodber ngodber commented May 19, 2021

Implement Multipole source and refactor BaseSrc for em.Static so that all source types are simply interfaces. PR also includes a number of minor bug fixes.

@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #1007 (d4d6522) into main (811209d) will decrease coverage by 0.01%.
The diff coverage is 77.42%.

@@            Coverage Diff             @@
##             main    #1007      +/-   ##
==========================================
- Coverage   79.14%   79.13%   -0.02%     
==========================================
  Files         158      158              
  Lines       20934    20954      +20     
==========================================
+ Hits        16569    16581      +12     
- Misses       4365     4373       +8     
Impacted Files Coverage Δ
SimPEG/directives/directives.py 73.34% <ø> (ø)
SimPEG/electromagnetics/analytics/DC.py 43.82% <0.00%> (ø)
...EG/electromagnetics/analytics/FDEMDipolarfields.py 8.82% <0.00%> (ø)
...imPEG/electromagnetics/frequency_domain/sources.py 86.81% <0.00%> (ø)
...omagnetics/natural_source/utils/plot_data_types.py 2.66% <0.00%> (ø)
...lectromagnetics/natural_source/utils/plot_utils.py 37.96% <ø> (ø)
...agnetics/static/induced_polarization/simulation.py 97.50% <0.00%> (ø)
SimPEG/electromagnetics/time_domain/sources.py 88.01% <0.00%> (ø)
SimPEG/fields.py 96.55% <0.00%> (ø)
SimPEG/potential_fields/gravity/simulation.py 83.20% <ø> (ø)
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 811209d...d4d6522. Read the comment docs.

@ngodber
Copy link
Contributor Author

ngodber commented May 20, 2021

@jcapriot, requesting review.

Cheers!

@ngodber
Copy link
Contributor Author

ngodber commented Jul 10, 2021

@jcapriot I've checked and the current test failure is the same as HEAD on master and so is unrelated. Is there any issues against getting this merged?

@jcapriot jcapriot requested a review from sgkang August 4, 2021 18:17
@sgkang
Copy link
Contributor

sgkang commented Aug 10, 2021

Hi @ngodber, thanks for your contribution!
It seems the major change that you have made was generalizing eval function for Pole and Dipole sources in BaseSrc.
That part looks fine to me. However, I was not sure what was Mutipole source is for?
Can you explain what your intention was for?

Copy link
Contributor

@sgkang sgkang left a comment

Choose a reason for hiding this comment

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

Hi @ngodber, thanks for your contribution!
Overall, your changes looking good to me. If you can explain a bit more about the Mulpole class that will be helpful. My guess is the case when there are more than two electrodes, but not exactly sure yet.

SimPEG/maps.py Outdated Show resolved Hide resolved
SimPEG/potential_fields/base.py Outdated Show resolved Hide resolved
@ngodber
Copy link
Contributor Author

ngodber commented Aug 10, 2021 via email

@ngodber ngodber force-pushed the dc_general_source branch 2 times, most recently from 2a1a8ea to 9f2d275 Compare February 4, 2022 13:02
@ngodber ngodber closed this Feb 4, 2022
@ngodber ngodber reopened this Feb 4, 2022
@ngodber ngodber requested a review from sgkang February 4, 2022 13:58
@ngodber ngodber closed this Feb 4, 2022
@ngodber ngodber reopened this Feb 4, 2022
@ngodber
Copy link
Contributor Author

ngodber commented Feb 4, 2022

@sgkang codecov is somehow bugged now, I added tests to improve coverage and somehow its gone way down!?

…reated via other implementations (two sources and one sink). Simulates orthogonally orientated array.
@ngodber
Copy link
Contributor Author

ngodber commented Feb 26, 2022

@sgkang Updated test to make to truly multipole (rather than a 'multipole' that was in fact a dipole).

@ngodber
Copy link
Contributor Author

ngodber commented Apr 15, 2022

@lheagy this is the PR I was referring too in the meeting.

Copy link
Contributor

@sgkang sgkang left a comment

Choose a reason for hiding this comment

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

Looking good to me. Thanks for editing the test.
Please pull the latest main branch, and we could move on.

@ngodber ngodber closed this Apr 23, 2022
@ngodber ngodber reopened this Apr 23, 2022
@jcapriot jcapriot merged commit b3155c7 into simpeg:main Apr 29, 2022
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.

None yet

3 participants