Skip to content
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

Adaptation of read_interpolated_variable() to the AMR #140

Merged
merged 5 commits into from
Jun 21, 2021

Conversation

maximegrandin
Copy link
Contributor

This is an attempt to adapt read_interpolated_variable() to the AMR. The main modification is related to the case when the coordinates of several points are given, as the original procedure (sorting data according to the cellids and equating cellid and index) cannot work with the AMR.
Note: Currently (i.e., in current master version), get_cell_neighbor() searches for the neighbour cell(s) at the same refinement level as the base cell. This "works" since the correct cellid is found (no matter the neighbour's refinement level) at the imposed distance (= base-cell refinement-level resolution) from the base cell, but this does not handle correctly cases when the neighbouring cell is at a higher refinement level (i.e., we end up taking the data from the "second neighbour" cell by taking too large a step). This is not critical – it results in smoothing of the data, I think, but shouldn't in theory create artifacts – but we might want to implement a better solution at some point.

Copy link
Contributor

@markusbattarbee markusbattarbee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a good point to add a check to exit if the variable is registered as an FSgrid variable. (At the start, check if it starts with `fg_´, redirect to read_interpolated_fsgrid_variable(), and add that dummy function with a warning of it being not yet implemented.

Interpolation in AMR space when there's a refinement level change is tricky. I'd recommend adding a check to see if the reflevels of lower_cell_id and upper_cell_id are different - if they are, continue as now but also print a warning to the user.

ngbrvalues[x,y,z,:] = sorted_variable[id]
# id=int(self.get_cell_neighbor(lower_cell_id, [x,y,z] , periodic) - 1) # Mind the -1 offset to access the array!
cellid_neighbor = int(self.get_cell_neighbor(lower_cell_id, [x,y,z] , periodic))
ngbrvalues[x,y,z,:] = whole_variable[np.argwhere(whole_cellids==cellid_neighbor)[0][0]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The page
https://numpy.org/doc/stable/reference/generated/numpy.argwhere.html
states that argwhere shouldn't be used for indexing, but use np.nonzero instead. Worth looking into, and could give cleaner code?

@maximegrandin
Copy link
Contributor Author

The suggested changes have been implemented, and this has been tested.
A cleaner handling of AMR refinement boundaries is not trivial and is left for future work.

@markusbattarbee
Copy link
Contributor

One more small change request:

   if name[0:3] == 'fg_':
         self.read_interpolated_fsgrid_variable(name, coordinates, operator, periodic)
         return -1

should be

   if name[0:3] == 'fg_':
         return self.read_interpolated_fsgrid_variable(name, coordinates, operator, periodic)

@markusbattarbee markusbattarbee merged commit dcbbca1 into fmihpc:master Jun 21, 2021
Sign up for free to 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