Resolve "(Learner1D) add possibility to use the direct neighbors in the loss"

There might be an automatic way of determining that loss_depends_on_neighbors = True such that the user wouldn't have to specify it. We could of course also change the API such that neighbors are always passed, which is a very cheap operation to, the updating of the neigboring losses is a bit expensive to always do though. Probably @anton-akhmerov and @jbweston have an opinion.

Well, the only way would be to either make it a seperate learner (like CurvatureLearner1D) or make it true by default. As I cannot think of an easy way to automatically determine it.

Unless one would use a decorator around a loss or something that indicates to the learner we need the neighbours:

e.g.

@uses_neighbours
def loss(interval, neighbours, scale, all_points):
    return "something"

added 1 commit

be4765b7 - remove 'loss_depends_on_neighbours' and replace by 'nn_neighbors'

Compare with previous version

I removed loss_depends_on_neighbours and replaced it by nn_neighbors (N nearest neighbors). This makes the code work with any number of neighbors. Now the new triangle loss even works with nn_neigbors=0. I also added that we pass the neighbors in all loss_per_interval functions.

added 1 commit

9d49bc09 - rename 'function_values' to 'data' because it's more obvious what it is

Compare with previous version

added 2 commits

0b397ad2 - remove 'loss_depends_on_neighbours' and replace by 'nn_neighbors'
7fdd00a4 - rename 'function_values' to 'data' because it's more obvious what it is

Compare with previous version

added 2 commits

69334d61 - remove 'loss_depends_on_neighbours' and replace by 'nn_neighbors'
544b0dc6 - fix typo

Compare with previous version

added 1 commit

0e0dd5da - rename 'function_values' to 'data' because its more obvious

Compare with previous version

added 1 commit

6d7da716 - simplifications

Compare with previous version

@Jorn I am really confused:

loss = loss_per_interval=adaptive.learner.learner1D.get_curvature_loss()
learner1 = adaptive.Learner1D(f, bounds=(-1, 1), nn_neighbors=0, loss_per_interval=loss)
learner2 = adaptive.Learner1D(f, bounds=(-1, 1), nn_neighbors=1, loss_per_interval=loss)
runner = adaptive.BlockingRunner(learner1, goal=lambda l: l.npoints > 400, log=True)
adaptive.runner.replay_log(learner2, runner.log)
assert learner1.data == learner2.data
assert learner1.losses == learner2.losses

This implies that the losses of the neighbors don't really need to be updated, or not?

edit: I understand this now. This happens on my computer with just 2 cores, however on io (with 48 cores) there are more interpolated intervals, so there an AssertionError is raised.

I would expect that the second assertion should fail for any function that is not a straight line, regardless of the number of cores.

added 1 commit

f9032bc4 - take the correct neigbours into account

Compare with previous version

@basnijholt I found why it goes wrong and solved it in: f9032bc4

added 1 commit

aff1f1e7 - fix deprecation warning and simplify

Compare with previous version

Good catch!

I think it only needs some more documentation and then this is ready :)

added 1 commit

1d637cc8 - introduce 'fast_det' for 2x2 and 3x3 matrices

Compare with previous version

mentioned in merge request !132

added 1 commit

443a68c8 - fix tests

Compare with previous version

resolved all discussions

added 1 commit

94419343 - added curvature docs

Compare with previous version

added 1 commit

86363644 - introduce 'annotate_with_nn_neighbors' decorator

Compare with previous version

@Jorn, I just figured that the user shouldn't need to worry about setting nn_neighbors him/herself, so I added the annotate_with_nn_neighbors decorator in 86363644.

What do you think?