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

Quick work-around; unused max cells parameter #217

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

Conversation

agzimmerman
Copy link

@agzimmerman agzimmerman commented Jul 22, 2016

This is a work-around for issue #216 .

The 'max number of cells' parameter in the refinement section does not work for my simple code that uses the Poisson problem interface. I have not found the root cause. This commit is a quick work-around that instead:

  1. adds a new parameter max_cells to the pi-DoMUS interface.
  2. checks the number of active cells against this parameter in should_solver_restart

Such a parameter seems at least as relevant as already existing parameters such as 'kelly threshold' and 'max iterations', so maybe this is actually what we want. This 'work-around' commit violates the concept of 'do not repeat yourself', so I would call this an open issue.

…t work for my simple code that uses the poisson problem interface. I have not found the root cause. This commit is a quick work-around that instead adds a new parameter max_cells to the pi-DoMUS interface. Such a parameter seems at least as relevant as already existing parameters such as 'kelly threshold' and 'max iterations', so maybe this is actually what we want. This 'work-around' commit violates the concept of 'do not repeat yourself', so I would call this an open issue.
@asartori86
Copy link
Contributor

Hi @alexanderzimmerman, thanks for your contribution.

However, the ParsedGridRefinement already features the max_cells parameter. Could you try if this works (without your patch)? By default the section for grid refinement is as follows

subsection Refinement
  set Bottom fraction                        = 0.100000
  set Maximum number of cells (if available) = 0
  set Order (optimize)                       = 2
  set Refinement strategy                    = fraction
  set Top fraction                           = 0.300000
end

You should change the Refinement strategy to number and specify the Maximum number of cells

@luca-heltai
Copy link
Contributor

Can you Move this workaround in deal2lkit instead of pi-domus? I think deal2lkit already has a similar logic behind its max_cells argument. There may well be a bug there if that does not work, but it is supposed to... If it doesn't, the right way is not to fix pi-domus, but to fix deal2lkit...

@luca-heltai
Copy link
Contributor

And, by the way, thanks!

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