Adding sample_weight to talos.Scan()

[ghost]

Hi,

I have added a sample_weight arugment to Scan(). I realise that it was possible to use the sample_weight argument of the Keras model fit method if one were to pass x_val and y_val, however this would require defining the weights outside of the model function which I don't find very satisfying.

The weights have been added as an argument to Scan() as opposed to an element of params as I do not consider the sample weights to be a hyper-parameter and it would seem to conflict with your design philosophy especially regarding the automated parameters (you can't automatically generate sample weights for example).

This change has resulted in the signature of model which is being passed to Scan() being dependent on the arguments passed to Scan(). I appreciate that this adds some complexity but if the user does not touch the new argument the original behaviour should be preserved. This has been explained in the documentation.

Let me know what you think of this featue (it is essential for my work!). Maybe you can think of a more elegant implementation?

Thanks, Tom

Sanity

• [x] I'm aware of the implications of the proposed changes
• [x] Code is PEP8
• [x] I'm making the PR to master

Docs

I've made changes to the docs but assume the following are not satisfied automatically.

• [ ] Docs are updated
• [ ] Docs version is correct (index.html and _coverpage.md)

Tests

I am on a windows machine and the test_local.sh script doesn't seem to work for me, so I just ran some examples on my own dataset that seemed to work just fine.

• [x] Changes have gone through actual use testing
• [ ] All local tests have passed (run ./test.sh in /talos)
• [x] Tests have been updated to reflect the changes

---

[pep8speaks]

Hello @charmander35! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file test/commands/test_scan.py:

Line 29:19: E124 closing bracket does not match visual indentation Line 75:80: E501 line too long (101 > 79 characters) Line 76:33: E124 closing bracket does not match visual indentation Line 93:80: E501 line too long (101 > 79 characters) Line 94:33: E124 closing bracket does not match visual indentation

Comment last updated at 2020-06-25 12:05:12 UTC

[ghost]

• Have addressed non-compliance with pep8.
• Not sure what to do about iris_model now having too many args.
• Not sure why numpy.random can't be imported but assuming this is an environment issue with the runner or something similar.
• The line which is too long was not added by me.

[ubrg]

Hello,

What about adding a fixed_params parameter in the signature of the Scan() and in the signature of the model function ? This fixed_params parameter could be a dictionary including all fixed parameters used by the model function, which do not require any optimization (such as sample_weight).

That way the signature of Scan() and modelfunction won't have to be modified anymore, if we want to add a new parameter unrelated to the optimization process.

[ghost]

Agreed that would be more elegant. I had thought of doing something similar but hadn't considered that it would also result in being able to keep the same signature of Scan() so it's probably the least distruptive way to add this feature. I'll push some more commits with that implemented when I get the chance.