-
Notifications
You must be signed in to change notification settings - Fork 13
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
Tests mesh #255
Tests mesh #255
Conversation
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.
First set of feedback.
Co-authored-by: Hans Fangohr <[email protected]>
Co-authored-by: Hans Fangohr <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Hans Fangohr <[email protected]>
for more information, see https://pre-commit.ci
The constructor does not check if the cell size exceeds the actual region (relevant code ends at https://github.com/ubermag/discretisedfield/pull/255/files#diff-c61c4320b3a6f255741f90a4610a5b2aae26f9698465b9fabab7e35b43b3938eL218) Here is an example: In [57]: mesh = df.Mesh(region=df.Region(p1=0, p2=1e-3), cell=1)
In [58]: mesh
Out[58]: Mesh(Region(pmin=[0.0], pmax=[0.001], dims=['x'], units=['m']), n=[0])
In [59]: mesh.dx
/Users/fangohr/Documents/GitHub/discretisedfield/discretisedfield/mesh.py:294: RuntimeWarning: divide by zero encountered in divide
return np.divide(self.region.edges, self.n).astype(float)
Out[59]: inf We do check if the cell size is too large: In [60]: mesh = df.Mesh(region=df.Region(p1=0, p2=1e-2), cell=1)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
Cell In[60], line 1
----> 1 mesh = df.Mesh(region=df.Region(p1=0, p2=1e-2), cell=1)
File ~/Documents/GitHub/discretisedfield/discretisedfield/mesh.py:214, in Mesh.__init__(self, region, p1, p2, n, cell, bc, subregions, attributes)
210 rem = np.remainder(self.region.edges, cell)
211 if np.logical_and(
212 np.greater(rem, tol), np.less(rem, np.subtract(cell, tol))
213 ).any():
--> 214 raise ValueError(
215 "Region cannot be divided into "
216 f"discretisation cells of size {cell=}."
217 )
218 self._n = np.divide(self.region.edges, cell).round().astype(int)
220 elif n is not None and cell is None:
221 # scalar data types for 1d regions
ValueError: Region cannot be divided into discretisation cells of size cell=[1]. but that checks seems to fail if the discrepancy between |p2-p1| and the cell size becomes too large. I could create an issue for this so we can move on with this PR? |
Overall this looks clean and very nicely done. |
The bug with cell size > region size has been fixed in 5cdc0bc |
No description provided.