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

dem_error w/ phase velocity: rm all-zero column in design matrix #905

Merged
merged 2 commits into from
Nov 12, 2022

Conversation

yunjunz
Copy link
Member

@yunjunz yunjunz commented Nov 11, 2022

Description of proposed changes

  • dem_error.py --phase-velocity: remove the all-zero column in the design matrix G and the all-one column in the design matrix G0, as suggested by @mirzaees in abnormal large residual from dem_error for timeseries_rms #892. This is because the constant intercept term of the polynomial is not estimated in the phase velocity minimization scenario. Including this all-zero column in G could sometimes introduce abnormally large eigenvalues in the least-squares inversion, resulting in an unrealistic TS residual.

Reminders

  • Pass Codacy code review (green)
  • Pass Pre-commit check (green)
  • Pass Circle CI test (green)
  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • If modifying functionality, describe changes to function behavior and arguments in a comment below the function declaration.

+ dem_error.py --phase-velocity: remove the all-zero column in the design matrix G and the all-one column in the design matrix G0, as the constant intercept term is not estimated in the phase velocity minimization scenario. Including this all-zero column in G could sometimes introduce abnormally large eigenvalues in the least-squares inversion, resulting in unrealistic TS residual.
Copy link
Collaborator

@mirzaees mirzaees left a comment

Choose a reason for hiding this comment

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

Thank you @yunjunz! It looks good.
The only point that we should not forget is that timeseries residuals in line 266 lacks a constant if using phase velocity and results large residuals, hence not reliable.
It will be resolved after moving this part to velocity estimation

@yunjunz yunjunz merged commit 16f4596 into insarlab:main Nov 12, 2022
@yunjunz yunjunz deleted the dem_error branch November 12, 2022 06:16
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.

2 participants