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

Amplitude regularization #1103

Merged
merged 79 commits into from Jun 6, 2023
Merged

Conversation

domfournier
Copy link
Contributor

@domfournier domfournier commented Sep 15, 2022

New implementation for MVI sparse inversion to keep the problem (mostly) linear.

  • Implementation of VectorAmplitude regularization

$\phi_s = || \mathbf{W} \mathbf{P} \mathbf{a}(m - m_{ref}) ||_2^2$

  • Implementation of Group map as extension of Wire with derivatives of wires returned as a list.

Solution with amplitude regularizer

Sparse_MVI

@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Merging #1103 (ff23ad4) into main (e666ab0) will decrease coverage by 5.38%.
The diff coverage is 92.42%.

@@            Coverage Diff             @@
##             main    #1103      +/-   ##
==========================================
- Coverage   81.85%   76.48%   -5.38%     
==========================================
  Files         163      163              
  Lines       24570    24674     +104     
==========================================
- Hits        20111    18871    -1240     
- Misses       4459     5803    +1344     
Impacted Files Coverage Δ
SimPEG/directives/directives.py 79.65% <72.72%> (-3.80%) ⬇️
SimPEG/regularization/base.py 83.48% <90.00%> (-11.34%) ⬇️
SimPEG/regularization/vector.py 69.71% <96.38%> (-30.29%) ⬇️
SimPEG/maps.py 68.56% <100.00%> (-17.41%) ⬇️
SimPEG/objective_function.py 74.03% <100.00%> (-21.60%) ⬇️
SimPEG/regularization/__init__.py 92.68% <100.00%> (-2.32%) ⬇️
SimPEG/regularization/sparse.py 91.33% <100.00%> (-5.42%) ⬇️

... and 30 files with indirect coverage changes

@domfournier
Copy link
Contributor Author

@orerocks All your dreams will come true.

@domfournier domfournier closed this Oct 8, 2022
@domfournier domfournier reopened this Oct 11, 2022
# Conflicts:
#	SimPEG/directives/directives.py
#	SimPEG/directives/pgi_directives.py
#	SimPEG/maps.py
#	SimPEG/regularization/sparse.py
@jcapriot
Copy link
Member

jcapriot commented Apr 20, 2023

Hey Dom, I think we're both finding some similar choke points in my cross-reference vector regularization (just opened it at #1214), and this one.

  • Had to deal with the UpdateSensitivityWeights directive to handle vector valued models.
    • Mine unconditionally reshapes the output of reg.mapping * jtj_diag to be a (n_cells, -1) array, so then we can always divide by the mesh volume as vol[:, None]
  • Needed a way to verify the model weight sizes.
    • Mine allowed for (n_cells,), (n_cells*dim,), and (n_cells, dim) shapes. Then I conditionally handled them in the _W function.

I think we can split a few common things into a BaseVectorRegularizer class, and maybe have them all live inside a SimPEG.regularization.vector.py module?

Some design thoughts, would it be easier if we didn't have to specify a wiremap for the three components in this regularizer? (The simulations already except models like this without mapping them to the different components, so the model should already be in a n_cells * dim array), then it can get reshaped within the calls?

One detail though, why are you passing the model difference (dm) to the mapping.deriv in f_m_deriv? Pretty sure it should just be the m, unless you had a specific reason (this error would only have shown up in a non-linear mapping).

P.S. Can you merge main in? I think there's some duplicate fixes.

@domfournier
Copy link
Contributor Author

domfournier commented Apr 20, 2023

@jcapriot Yea I saw your modifications to the UpdateSensWeights. I like yours better.

Looks like we may be able to share some components indeed. Not sure how you want to proceed?

The wire mapping is kind of nice to keep the operations close to what the linear algebra looks like. And since Wires doesn't have derivatives, I thought we could just implement it in here.

Good point on dm - typo. Won't change anything anyway since the deriv of a wire just returns the projection.

I will merge main in.

@domfournier
Copy link
Contributor Author

@jcapriot I refactored using your base class.

@domfournier domfournier requested a review from jcapriot May 17, 2023 17:11
@domfournier
Copy link
Contributor Author

@jcapriot Ready for another review.

@jcapriot jcapriot merged commit fb26cea into simpeg:main Jun 6, 2023
15 of 16 checks passed
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