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

[MRG] Temporal weighting #31

Merged
merged 8 commits into from
May 19, 2020
Merged

[MRG] Temporal weighting #31

merged 8 commits into from
May 19, 2020

Conversation

mstimberg
Copy link
Member

This adds a simple temporal weighting factor to MSEMetric. Instead of t_start you can provide t_weights, a vector of values of the same size as the trace that gets multiplied with the error (i.e. scales the squared error in the case of MSEMetric ). For simplicity, it cannot be combined with t_start.

Closes #22

@mstimberg mstimberg changed the title Temporal weighting [MRG] Temporal weighting Apr 23, 2020
@mstimberg
Copy link
Member Author

Somewhat forgot about this PR. @romainbrette, what do you think?

@romainbrette
Copy link
Member

It runs, but does it apply also to refine?

There seems to be some incompatibility with latest scikit-optimize
@mstimberg
Copy link
Member Author

It runs, but does it apply also to refine?

Oops, good catch! No it didn't, but now it does :)

@romainbrette
Copy link
Member

What does the error correspond to now? mean(t_weight*error)?

@mstimberg
Copy link
Member Author

What does the error correspond to now? mean(t_weight*error)?

Yes. I now realize that we should maybe make this clearer in the documentation. Weighting the first part of the trace as 0 or using t_start is similar, but it will not give the same result for the error – in the former case the error is considered 0 (but included in the mean) and in the latter it is simply ignored.

We could renormalize things by considering the total weight (so only the relative weights would matter)?

@romainbrette
Copy link
Member

for example with something like t_weights = t_weights/(sum(t_weights)*dt) ?

@romainbrette
Copy link
Member

En tout cas ça marche!

@romainbrette
Copy link
Member

Ah non ça ne doit pas être exactement ça

@romainbrette
Copy link
Member

(la normalisation je veux dire)

@mstimberg
Copy link
Member Author

I guess

t_weights = len(t_weights)*t_weights/sum(t_weights)

would work?

@romainbrette
Copy link
Member

yes that seems rights (ie t_weights/mean(weights)).

@mstimberg
Copy link
Member Author

I implemented the normalization of the weights. This makes sure that using t_start generates the same error as using t_weights with zero for time points < t_start and any kind of constant value for the other time points.
Currently, the weights weigh the squared error, or in other words the residual is weighted by the square root of the provided weight. Not sure whether this is the best approach, especially given that the normalization scales the residual and not the squared error.

@mstimberg
Copy link
Member Author

Discussed with @romainbrette and decided that applying the weighting to the squared error is ok. After all, users will chose this value somewhat arbitrarily anyway. Going ahead with the merge.

@mstimberg mstimberg merged commit 73102d0 into master May 19, 2020
@mstimberg mstimberg deleted the temporal_weighting branch May 19, 2020 15:08
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.

Temporal weighting of error
2 participants