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

Level specific wave tolerance #241

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

akshaysripada
Copy link

Wave tolerance is now set to be level specific, very similar to the speed refinement. If the number of wave tolerance values are less than the max amr level specified, the last value of the wave tolerance is used for the remainder of the levels.

@mandli
Copy link
Member

mandli commented Jan 18, 2017

Does this then work with only a scalar specification of the wave_tolerance in the setrun.py files?

@akshaysripada
Copy link
Author

akshaysripada commented Jan 18, 2017

Yes it will work for a scalar representation since refinement_module.f90 will read the values in refinement.data file for the wave tolerances and store it in an array regardless of number of values provided.

enddo
! Check if wave tolerance array size is smaller than max level
! then refine with the last wave tolerance value for the rest of the levels
if (size(wave_tolerance) < mxnest-1) then
Copy link
Member

Choose a reason for hiding this comment

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

I might suggest pre-processing this so that the wave_tolerance array is always at least mxnest so you can simplify this logic and not replicate the checks.

@rjleveque
Copy link
Member

@mandli: This wave_tolerance logic is based on how speed_tolerance is handled, but I don't understand how this is supposed to be used exactly.

I would have thought that speed_tolerance[m] is only used to determine whether to refine a grid at level m to level m+1. But instead, the way it is written to loop over m=1,mxnest, I think that level L is refined to level L+1 if the speed is at least as great as any of the values speed_tolerance[m] for m >= L.

Is this really what's intended? If the tolerances are non-decreasing then I think it has the behavior I expect but if speed_tolerance[m] > speed_tolerance[m+1] for some m then I think speed_tolerance[m] is never actually used as a cutoff at any level and so this is just confusing.

Other minor problems with flag2refine.f90:

  • tab characters rather than blanks starting some lines
  • Print statements should be removed.

Also we probably want to modify data.py to initialize wave_tolerance consistent with how speed_tolerance is initialized, which is currently self.add_attribute('speed_tolerance',[1.0e12]*6), but actually I think it would be better to leave self.add_attribute('wave_tolerance',1.0e-1) alone and change speed_tolerance to a scalar, which works fine and seems more logical than setting the same value 6 times (when the user might have more than 6 levels).

@mandli
Copy link
Member

mandli commented May 8, 2017

@akshaysripada Can you take care of some of these suggestions from Randy? I am a bit confused as well about the way you implemented this but I need to look at a bit more before I have a suggestion.

@mandli
Copy link
Member

mandli commented May 9, 2017

So looking carefully at the code I think there are a couple of modifications and one question that @rjleveque brought up:

  • The loops that determine the particular tolerance to use need to be outside of the loops over cells as these are only needed once for each grid.
  • Move the if statement into the refinement_module where this can be done once.
  • On the latter if the criteria are not monotonic I am not sure what the best thing to do is. One reason we only checked this once was to reduce the amount of checking but we could always look for the threshold for all levels just to make sure.
  • Need to add the data.py changes.

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.

3 participants