Skip to content

pass value_scale to the LearnerND's loss_per_simplex function - #219

Merged
jbweston merged 1 commit into
masterfrom
value_scale_ND
Sep 20, 2019
Merged

pass value_scale to the LearnerND's loss_per_simplex function#219
jbweston merged 1 commit into
masterfrom
value_scale_ND

Conversation

@basnijholt

@basnijholtbasnijholt commented Sep 20, 2019

Copy link
Copy Markdown
Member

Currently, the LearnerND cannot access the unscaled value in the loss_per_simplex.

This PR fixes this by passing the scale. This fixes a regression introduced when we added scaling.

Now the following code:

importnumpyasnpimportadaptiveadaptive.notebook_extension()
defisoline_loss_function(y_iso, sigma, priority=1):
fromadaptive.learner.learnerNDimportdefault_lossdefgaussian(x, mu, sigma):
returnnp.exp(-(x-mu) **2/sigma**2)
defloss(simplex, values, value_scale):
distance=np.mean([abs(y_iso*value_scale-y) foryinvalues])
returnpriority*gaussian(distance, 0, sigma) +default_loss(simplex, values, value_scale)
returnlosslevel=0.5loss=isoline_loss_function(y_iso=level, sigma=1, priority=0.3)
deff(xy):
x, y=xyreturnx**2+y**2/2+y**3learner=adaptive.LearnerND(
f,
bounds=[(-1, 1), (-1, 1)],
loss_per_simplex=loss,
)
adaptive.runner.simple(learner, goal=lambdal: l.npoints>1000)
%optsImage [colorbar=True]
learner.plot_isoline(level, tri_alpha=0.3)

produces
image

instead of

image

@jbweston

Copy link
Copy Markdown
Contributor

Seems ok, don't know why tests are failing

@jbweston

Copy link
Copy Markdown
Contributor

@basnijholt the test failures are genuine.

> loss_curvature = triangle_loss(simplex, values, neighbors, neighbor_values)
E TypeError: triangle_loss() missing 1 required positional argument: 'neighbor_values'

Threre may be others

@basnijholt

Copy link
Copy Markdown
MemberAuthor

Thanks! I've already pushed a fix.

@basnijholt

Copy link
Copy Markdown
MemberAuthor

@jbweston merge if you are OK with this 👍

@jbweston
jbweston merged commit fc297c3 into masterSep 20, 2019
@basnijholt
basnijholt deleted the value_scale_ND branch September 20, 2019 12:21
@basnijholtbasnijholt mentioned this pull request Oct 7, 2019
Sign up for freeto 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.

2 participants

@basnijholt@jbweston