-
Notifications
You must be signed in to change notification settings - Fork 86
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
Added Latin Hypercube sampling #216
Conversation
added chebyshev2-noisy method for generators |
neurodiffeq/generators.py
Outdated
elif method == 'latin-hypercube': | ||
x = _latin_hypercube(xy_min[0], xy_max[0], grid[0]) | ||
y = _latin_hypercube(xy_min[1], xy_max[1], grid[1]) | ||
x = lambda: _latin_hypercube(xy_min[0], xy_max[0], grid[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to assign x
and y
as functions. The following line calling torch.meshgrid(x, y, indexing='ij')
will likely fail, unless there's some hacky trick in pytorch I'm not familiar with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done as suggested!
neurodiffeq/generators.py
Outdated
elif method == 'latin-hypercube': | ||
self.examples = _latin_hypercube(t_min, t_max, size) | ||
self.getter = lambda: self.examples | ||
self.examples = lambda: _latin_hypercube(t_min, t_max, size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't want to return static (fixed) points every time self.getter()
is called, we can combine both lines into
self.getter = lambda: _latin_hypercube(t_min, t_max, size)
and skip defining self.examples (or assigning it a dummy value for consistency). It's confusing to see self.examples
as a function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed self.examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another 2 minor changes and I think we're good to go.
No description provided.