-
Notifications
You must be signed in to change notification settings - Fork 89
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
feat: hypre improvements #3339
feat: hypre improvements #3339
Conversation
Here is a quick test I ran on a version of SPE10 with burdens, compositional poromechanics, though with homogeneous properties, (so its just a high perm slab between two low perm boxes). Should be about 40 million DoF, and I ran on Dane. Here were the results with develop a few weeks ago
Here are the results with the new Hypre
Note that this does not double the number of nodes for each test, I will do that now |
Old results for heterogeneous SPE10 compositional poromechanics on CPU (dane)
New results for heterogeneous SPE10 compositional poromechanics on CPU (dane)
|
…/GEOS into feature/paludettomag1/hypre-sdc
…er in several functions
…/GEOS into feature/paludettomag1/hypre-sdc
HYPRE_SetUmpirePinnedPoolName( "HYPRE_PINNED" ); | ||
#endif | ||
|
||
HYPRE_SetLogLevel( getenv( "HYPRE_LOG_LEVEL" ) ? atoi( getenv( "HYPRE_LOG_LEVEL" ) ) : 0 ); |
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.
can it be set by linear solver log level?
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.
Yes, but I did this way because the information provided here is mainly for developers. Other libraries such as umpire and rocsparse work this way as well (using env variable). I am open to change the strategy here if needeed
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.
sounds reasonable, thanks, no need to change
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.
let's not use environment variables please.
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.
I can remove this. What is the issue with env vars again?
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.
I personally don't like them. It's hard to keep track of who sets them. Also we don't really do it for anything else in GEOS.
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.
Here's a place where it's being used:
int columns = getenv( "COLUMNS" ) ? atoi( getenv( "COLUMNS" )) : 120; |
HYPRE_LOG_LEVEL
is an option supposed to be used by developers only. It's very convenient to turn it on/off if it's a env var wrt a XML input. I don't see any harm with using env var specifically, but I'm happy to remove it if that's the best solution
@castelletto1 @CusiniM @rrsettgast Can you guys take it from here? It seems there's nothing else I can do. Thanks! |
is this basically ready? I mean it clearly needs an LvArray PR and a rebaseline but is it ready otherwise? |
Right, it's ready. Needs a LvArray PR and LC build, which I can't do. Rebaseline is due to field value change ( |
separateComponents
option to mechanics solver setup in MGRaddCommaSeparators
to StringUtilitiesRequires GEOS-DEV/thirdPartyLibs#286