-
Notifications
You must be signed in to change notification settings - Fork 168
Renaming pyfunc to kernels #2490
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
base: v4-dev
Are you sure you want to change the base?
Changes from all commits
4b1e4e6
2497376
5fb6178
b1e2e2f
0bf0e42
d9c770d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -290,29 +290,6 @@ def from_particlefile(cls, fieldset, pclass, filename, restart=True, restarttime | |||||||||||||||||||||
| "ParticleSet.from_particlefile is not yet implemented in v4." | ||||||||||||||||||||||
| ) # TODO implement this when ParticleFile is implemented in v4 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def Kernel(self, pyfunc): | ||||||||||||||||||||||
| """Wrapper method to convert a `pyfunc` into a :class:`parcels.kernel.Kernel` object. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Conversion is based on `fieldset` and `ptype` of the ParticleSet. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Parameters | ||||||||||||||||||||||
| ---------- | ||||||||||||||||||||||
| pyfunc : function or list of functions | ||||||||||||||||||||||
| Python function to convert into kernel. If a list of functions is provided, | ||||||||||||||||||||||
| the functions will be converted to kernels and combined into a single kernel. | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| if isinstance(pyfunc, list): | ||||||||||||||||||||||
| return Kernel.from_list( | ||||||||||||||||||||||
| self.fieldset, | ||||||||||||||||||||||
| self._ptype, | ||||||||||||||||||||||
| pyfunc, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| return Kernel( | ||||||||||||||||||||||
| self.fieldset, | ||||||||||||||||||||||
| self._ptype, | ||||||||||||||||||||||
| pyfuncs=[pyfunc], | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def data_indices(self, variable_name, compare_values, invert=False): | ||||||||||||||||||||||
erikvansebille marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||
| """Get the indices of all particles where the value of `variable_name` equals (one of) `compare_values`. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -376,7 +353,7 @@ def set_variable_write_status(self, var, write_status): | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| def execute( | ||||||||||||||||||||||
| self, | ||||||||||||||||||||||
| pyfunc, | ||||||||||||||||||||||
| kernels, | ||||||||||||||||||||||
| dt: datetime.timedelta | np.timedelta64 | float, | ||||||||||||||||||||||
| endtime: np.timedelta64 | np.datetime64 | None = None, | ||||||||||||||||||||||
| runtime: datetime.timedelta | np.timedelta64 | float | None = None, | ||||||||||||||||||||||
|
|
@@ -390,10 +367,9 @@ def execute( | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| Parameters | ||||||||||||||||||||||
| ---------- | ||||||||||||||||||||||
| pyfunc : | ||||||||||||||||||||||
| Kernel function to execute. This can be the name of a | ||||||||||||||||||||||
| kernels : | ||||||||||||||||||||||
| List of Kernel functions to execute. This can be the name of a | ||||||||||||||||||||||
| defined Python function or a :class:`parcels.kernel.Kernel` object. | ||||||||||||||||||||||
| Kernels can be concatenated using the + operator. | ||||||||||||||||||||||
| dt (np.timedelta64 or float): | ||||||||||||||||||||||
| Timestep interval (as a np.timedelta64 object of float in seconds) to be passed to the kernel. | ||||||||||||||||||||||
| Use a negative value for a backward-in-time simulation. | ||||||||||||||||||||||
|
|
@@ -417,10 +393,12 @@ def execute( | |||||||||||||||||||||
| if len(self) == 0: | ||||||||||||||||||||||
| return | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if not isinstance(pyfunc, Kernel): | ||||||||||||||||||||||
| pyfunc = self.Kernel(pyfunc) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self._kernel = pyfunc | ||||||||||||||||||||||
| if isinstance(kernels, Kernel): | ||||||||||||||||||||||
| self._kernel = kernels | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| if not isinstance(kernels, list): | ||||||||||||||||||||||
| kernels = [kernels] | ||||||||||||||||||||||
| self._kernel = Kernel(kernels, self.fieldset, self._ptype) | ||||||||||||||||||||||
|
Comment on lines
+396
to
+401
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kernels input arg can't be Kernel object anymore. Also inverting the isinstance check would be more reliable
Suggested change
suggestion requires
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is related to the comment above #2490 (comment); there is a use-case for still accepting Kernels as input. unless we find another way to figure out whether the position_update_kernel has to be added or not |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if output_file is not None: | ||||||||||||||||||||||
| output_file.set_metadata(self.fieldset.gridset[0]._mesh) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
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.
? Not sure why this was added - slipped by me during the last review
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.
No, this is an addition after your reviews. We need this for one specific unit test (below) where a set.execute() is called in a for-loop
https://github.com/erikvansebille/Parcels/blob/32bcf937cb3776d3ac177d992ccf698de435954a/tests/test_particleset_execute.py#L253-L256
The point is that there is a difference between a function and a Kernel, because the latter has the
positionupdatekerneladded.https://github.com/erikvansebille/Parcels/blob/32bcf937cb3776d3ac177d992ccf698de435954a/src/parcels/_core/kernel.py#L283-L287
If a user would provide the function in the for-loop above, then the code above is triggered on each iteration, while if they provide the kernel object it's only triggered the first time
This difference between calling pest.execute with a function or a Kernel object may cause confusion to users, I now realise. But the code at the end of the kernel-loop is essential to make the updating of variables correct; this has been a lot of headaches in #2333, so I'm reluctant to change this again.
Something to think about when we're back from holidays....