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

Dennis & Schnabel algorithm for computing the differencing parameter #584

Closed
andrsd opened this issue Feb 14, 2014 · 18 comments
Closed

Dennis & Schnabel algorithm for computing the differencing parameter #584

andrsd opened this issue Feb 14, 2014 · 18 comments
Assignees
Labels
C: Framework Good first issue P: normal A defect affecting operation with a low possibility of significantly affects. T: task An enhancement to the software.

Comments

@andrsd
Copy link
Contributor

andrsd commented Feb 14, 2014

Add this PETSc options as default: -mat_mffd_type ds
maybe even create an option in the input file...

@ghost ghost assigned andrsd Feb 14, 2014
@friedmud
Copy link
Contributor

I think we should make this the default.

In my testing this never adds extra time... but always results in better convergence behavior.

@permcody
Copy link
Member

This is hilarious reading this several years in the future. I'm sure we'll only have to diff a few hundred tests.

@friedmud
Copy link
Contributor

Can't be that many tests using FD Jacobian...

@lindsayad
Copy link
Member

So changing the default from wp to ds leads to 54 test failures in our current suite

@permcody
Copy link
Member

permcody commented Sep 6, 2017

How about modules? 54 is not too bad and we could just set the fd type explicitly for those. If there are a ton in modules though, we might just have to boot on this.

@andrsd
Copy link
Contributor Author

andrsd commented Sep 6, 2017

We could still add the input file parameter for switching it, like we have solve_type.

@lindsayad
Copy link
Member

126 failures in module tests. Would having an input file parameter be much better than just specifying it in the petsc options?

@permcody
Copy link
Member

permcody commented Sep 6, 2017

I'm not terribly picky about PETSc specific options. Having an input parameter is nice, but PETSc guys like to just use petsc options directly so I can go either way.

One downside to just switching the default and updating all existing test is that MOOSE+modules is only the beginning. We are likely to run into even more problems in applications. Before you do anything else, push a PR and let CIVET show you just how bad the damage is for switching the default.

@lindsayad
Copy link
Member

Booting sounds good to me :-) But yea, I'll push a PR

@permcody
Copy link
Member

permcody commented Sep 6, 2017

Well one things for sure, we'll make a decision on whether or not we are going to close this issue rather than just leave it open. If this is going to be difficult, we'll document that and close this issue. We can't just let these sit open for years and years.

@andrsd
Copy link
Contributor Author

andrsd commented Sep 6, 2017

Would having an input file parameter be much better than just specifying it in the petsc options?

If we do an input file parameter and the PETSc options change, we fix the code and all input files work as before. If you keep it in petsc_options parameter and they change, then you need to go update all input files.

@fdkong
Copy link
Contributor

fdkong commented Sep 7, 2017

I am not a big fan to change the default PETSc setting in moose. There is the reason that WP is set as the default scheme in PETSc. WP is cheaper than DS because DS involves more collective operations.
http://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Mat/MATMFFD_DS.html#MATMFFD_DS

This is not significant when the number of processes is small, but it does matter when we are going to run a large number of processes (more than ten thousands).

Users can change this through a petsc option for the particular case, if the want.

@friedmud
Copy link
Contributor

friedmud commented Sep 7, 2017 via email

@fdkong
Copy link
Contributor

fdkong commented Sep 7, 2017

However: most (99.9%) of all MOOSE runs are done on less than 100
processors... so changing the default for MOOSE makes sense.

Potentially, I hope moose also runs smoothly with a large number of cores. And I guess WP fails for 1% of moose runs? If WP fails for a small number of moose runs, we can fix these individually.

BUT: we do have a huge history of having wp be the default... and changing
it now could be incredibly painful. So... I'm fine with leaving it as is.

@permcody
Copy link
Member

permcody commented Sep 7, 2017 via email

@lindsayad
Copy link
Member

Ok, we'll punt on making ds the default. But I'm going to work on making mffd_type an Executioner input parameter since I believe at least @andrsd would like this. Sound ok?

@andrsd
Copy link
Contributor Author

andrsd commented Sep 8, 2017

Sounds good to me.

@lindsayad
Copy link
Member

Closed by #9808

jarons pushed a commit to jarons/moose that referenced this issue Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Framework Good first issue P: normal A defect affecting operation with a low possibility of significantly affects. T: task An enhancement to the software.
Projects
None yet
Development

No branches or pull requests

5 participants