consider taking out sample weights

[jsadler2]

i'm considering taking out the sample weights functionality. I don't think anyone is using it and it does make things more complicated in calculating the metrics.

[jsadler2]

@janetrbarclay , @SimonTopp , @jzwart - Are any of you currently using this functionality or do you foresee yourself using it? The idea is that you can assign weights to individual losses. For example, in this way you could, in theory, emphasize or de-emphasize certain time periods or locations by increasing or decreasing their weights.

I built it in, but I don't actually use it and I feel like it kinda just is complicating things at this point.

[janetrbarclay]

I am not currently using it and haven't thought of doing that in the future. I imagine that since it's been written, it wouldn't be too hard to add it back in the future if someone wanted to use it, yes?

[SimonTopp]

I like the idea of being able to focus training on difficult time/space predictions, but in practice I'm not sure where it would fit into my planned workflow. I'd echo Janet's statement that you should feel free to remove it, and if for some reason I do want it down the road, I can go back and look at the code to re-incorporate it. Thanks for asking @jsadler2!

[jzwart]

I don't think it should be the default for calculating losses, but I have used it and will probably use it in the future. I use it for iterative training with updating paramters in DL and updating states with DA - there is uncertainty associated with state estimates for DA and I use that uncertainty to weight certain locations or time periods when re-training the DL model. I don't think I've used it for evaluation metrics during a test period though, only for training

[jsadler2]

Thanks for the input. Removing the weights from the normal loss functions has been removed in #104.

@jzwart - if you are still using this, we will need to revisit how to do this. When looking at the code, I actually realized that the weights were only minimally implemented in the loss functions anyway. They were only used here to change the y_true to np.nan anywhere the weight was 0. Then those values were not used in calculating the loss.

[jzwart]

Gotcha. Here's where I've used weights in the past - calculating weights and weighted rmse function

[SimonTopp]

Re-opening this issue because there's still significant code that implies the weights still work. Specifically the entire exclude segs pipeline is dependent on the weights. We should probably remove what is no longer functional now that the option for the weighted loss function is gone.

[SimonTopp]

@janetrbarclay, @jzwart , @aappling-usgs. Was just chatting with Janet and this came up again. All the exclude functionality doesn't propagate through to the loss function so right now it just creates the illusion that you're excluding things. Janet and I both agree that we should just remove it so it's not misleading. It's also somewhat redundant with specifying train/val/tst segs (which is incorporated and does work). Does anyone else have hot takes on this?