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

include the number of turns in the source initialization #1086

Merged
merged 28 commits into from Oct 14, 2022

Conversation

thibaut-kobold
Copy link
Contributor

@thibaut-kobold thibaut-kobold commented May 25, 2022

  • number of turns was an option in tdem circular loop. it was used for computing the moment but ignored at the field initalization
  • number of turns was absent from the fdem circular loop. it is added for consistency
  • In the end, I chose to avoid the geoana implementation to make this change backward compatible with older geoana versions

@thibaut-kobold thibaut-kobold changed the title include the number of loop in the source initialization include the number of turns in the source initialization May 25, 2022
@codecov
Copy link

codecov bot commented May 25, 2022

Codecov Report

Merging #1086 (1e6c3c3) into main (ca7af51) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1086      +/-   ##
==========================================
+ Coverage   79.04%   79.06%   +0.01%     
==========================================
  Files         137      137              
  Lines       20206    20224      +18     
==========================================
+ Hits        15972    15990      +18     
  Misses       4234     4234              
Impacted Files Coverage Δ
...imPEG/electromagnetics/frequency_domain/sources.py 90.70% <100.00%> (+0.23%) ⬆️
SimPEG/electromagnetics/time_domain/sources.py 89.86% <100.00%> (+0.12%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@thast thast requested review from jcapriot, lheagy and dccowan May 27, 2022 16:41
@thibaut-kobold
Copy link
Contributor Author

@jcapriot any recommendation to help this PR get merged? I tried adding test earlier but did not help (and I am in the field at the moment)

Copy link
Member

@lheagy lheagy left a comment

Choose a reason for hiding this comment

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

This looks good to me! I just made a small suggestion regarding naming

SimPEG/electromagnetics/frequency_domain/sources.py Outdated Show resolved Hide resolved
SimPEG/electromagnetics/frequency_domain/sources.py Outdated Show resolved Hide resolved
SimPEG/electromagnetics/frequency_domain/sources.py Outdated Show resolved Hide resolved
SimPEG/electromagnetics/time_domain/sources.py Outdated Show resolved Hide resolved
SimPEG/electromagnetics/time_domain/sources.py Outdated Show resolved Hide resolved
tests/em/fdem/forward/test_FDEM_analytics.py Outdated Show resolved Hide resolved
tests/em/tdem/test_TDEM_forward_Analytic.py Outdated Show resolved Hide resolved
thibaut-kobold and others added 16 commits August 9, 2022 16:50
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
# Conflicts:
#	SimPEG/electromagnetics/time_domain/sources.py
…turns

# Conflicts:
#	SimPEG/electromagnetics/time_domain/sources.py
thibaut-kobold and others added 6 commits October 4, 2022 19:09
# Conflicts:
#	SimPEG/electromagnetics/frequency_domain/sources.py
#	SimPEG/electromagnetics/time_domain/sources.py
#	tests/em/fdem/forward/test_FDEM_sources.py
@jcapriot jcapriot merged commit 853f53f into simpeg:main Oct 14, 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

4 participants