Conversation
Update `master` with changes from `v2.0`
publixsubfan
force-pushed
the
master
branch
2 times, most recently
from
6ce51e3 to
23c5e5e
Compare
|
Note: this should probably be reviewed together with mfem/mfem#2973 |
| echo "Map target to options" | ||
| if [[ "${{ inputs.target }}" == "int32" ]] | ||
| then | ||
| export hypre_options="--disable-fortran"; |
There was a problem hiding this comment.
Why is this removed?
(its purpose is to save a bit of build time as we don't need hypre's Fortran dependencies)
There was a problem hiding this comment.
When I looked into the Hypre documentation some time ago I couldn't find any reference to this option. Are you sure it is still a thing? We had a brief chat with @v-dobrev about it, and I believe he was also thinking that maybe we should remove it.
There was a problem hiding this comment.
It is indeed been many years since this was added, but I still see it here: https://github.com/hypre-space/hypre/blob/master/src/configure#L1514
Maybe it is not doing anything anymore? Part of the reason to use it was that otherwise one needed a Fortran compiler, which may not be available by default e.g. on a Mac.
If that's not an issue anymore, I am OK with removing it.
adrienbernede
left a comment
adrienbernede
left a comment
There was a problem hiding this comment.
I need some answers :)
adrienbernede
left a comment
adrienbernede
left a comment
There was a problem hiding this comment.
My main concern is the design comment. @kanye-quest if you agree with that, then maybe we can move forward and make the change, this has no impact on functionality.
|
@adrienbernede, can you take another look? |
adrienbernede
left a comment
adrienbernede
left a comment
There was a problem hiding this comment.
Looks good to me. Feel free to accept or reject the suggested comment.
Co-authored-by: Adrien Bernede <51493078+adrienbernede@users.noreply.github.com>
|
Thanks @kanye-quest ! |
4 participants
No description provided.