Semivariogram parameter fitting/optimization #251
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was
linked to
issues
Jun 18, 2024
…IP_semivariogram_fitting
leewujung
reviewed
Aug 17, 2024
leewujung
reviewed
Aug 17, 2024
leewujung
reviewed
Aug 17, 2024
leewujung
reviewed
Aug 17, 2024
leewujung
reviewed
Aug 17, 2024
leewujung
reviewed
Aug 17, 2024
leewujung
reviewed
Aug 17, 2024
There was a problem hiding this comment.
Hey @brandynlucca : thanks for the PR, good work! exciting to see the fitting functionality!
I have some inline comments, mostly for bundling things up so that the code would look a bit cleaner and easy to follow.
I read through all the main variogram functions called by Survey.fit_variogram but not the various variogram models.
Because I didn't read the variogram models, I also noticed that they do not have accompanying tests. I think we can do without them in this PR, but it would be important to add them to make sure that the calculations are implemented correctly. I'll create an issue for this and let's discuss how to tackle it more efficiently.
Update: Added issue #263 to track testing needs.
…IP_semivariogram_fitting
for more information, see https://pre-commit.ci
by creating separate parameter classes with validation steps inherent to them.
…nlucca/echopop into WIP_semivariogram_fitting
for more information, see https://pre-commit.ci
`typing.TypedDict` class approach
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…nlucca/echopop into WIP_semivariogram_fitting
for more information, see https://pre-commit.ci
storing expected test results
…nlucca/echopop into WIP_semivariogram_fitting
for more information, see https://pre-commit.ci
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A new
Survey-class method (fit_variogram(...)) has been added to allow for the non-linear least squares fitting of variogram parameters. This includes options for only fitting select parameters (instead of all), as well as modifying different parameter values (i.e.initial_values), bounds (i.e.lower_bounds,upper_bounds), and various arguments specific to the optimization methods/algorithm. These results can then be propagated over to theSurvey.kriging_analysis(..., best_fit_variogram = True)method.