-
Notifications
You must be signed in to change notification settings - Fork 191
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
hypre-multiprecision build #850
base: master
Are you sure you want to change the base?
Conversation
Thanks for all this work, Daniel! I'm trying to build hypre via
|
Also, the standard build
|
@victorapm , thanks. I rushed a few things in my PR. I will update with a fix soon. |
Thanks @oseikuffuor1! The standard build (mixed precision disabled) is working fine to me. I'm still seeing some problems with the mixed precision build. Apart from compilation warnings with
Note in the standard build, the
Maybe I'm missing something, please, let me know! |
hypre_scanf | ||
hypre_sprintf | ||
hypre_sscanf | ||
new_format |
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.
The functions.saved
file reveals something interesting. After looking at this list of functions, we can see that some do not start with hypre_
. This is a note that we should fix them @rfalgout
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.
Good catch, Victor! The free_format
and new_format
functions are intended to be static
(file scope only) so we should prepend the static
identifier in their definitions. I'm not sure how this affects the multiprecision stuff here (I haven't thought about it yet).
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.
Good points. For now, static functions are not converted. I think that should be fine, but we can discuss this further.
#mkmpdir: | ||
# ${MKDIR_P} ${MuP_OBJDIR} | ||
# | ||
ADD_OBJS_single = ${addprefix ${MuP_OBJDIR_single}/, ${notdir ${OBJS}}} |
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.
Most of these Makefile additions should go in Makefile.config
instead.
That should be doable based on a diff of the blas
and lapack
directories. The only issue there is the dlamch
compile, but in looking at the current master branch, the special compile "without optimization" for this file is broken anyway (it looks like it uses CFLAGS
just as in the normal build).
The diff with utilities
has many more differences that I don't yet understand (I need to dig into it), but I still think we should be able to isolate most of the makefile code in Makefile.config
, and that would help considerably with code maintenance.
I would be willing to give it a shot. That would force me to dig in further as well. :) Let me know @oseikuffuor1.
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.
@falgout, I agree we could move most of this into Makefile.config, as the approach is similar for many of the subdirectories. I also think it can be simplified or cleaned-up a bit. In addition, when we include the multiprecision methods, they will have to be built differently (only once) and so like the utilities directory, the resulting makefile will have slightly more diffs. Take a look at the seq_mv subdirectory on the mp-buil-all branch to see such an example. If you are interested in taking a pass on incorporating it within the Makefile.config file, let me know. Otherwise I can take a pass at it and iterate with you (and everyone else).
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.
Maybe this should be merged with the headers
script approach we are gradually adopting throughout hypre. That way the internal header file for each subdirectory in hypre is always done in the same way.
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.
@rfalgout Yes, I agree we can do that. Not all the subdirectories have this headers script, but perhaps this could force us to head in that direction.
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.
Related to the comment for script blas_func_header
, this file can just be named functions.saved
.
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 good to me
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 like the approach. Thanks for all the work, @oseikuffuor1! I'm excited to see how it all looks and works when extended to the rest of the repo!
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.
Looks good to me. (I didn't follow all the details). Thanks @oseikuffuor1 !
This PR updates various directories in hypre to support multiprecision build.
** This PR is currently defined for building hypre with configure. Cmake support will be added later. Also, GPU support is incomplete for now.