PTSCOTCH: Large refactor #16
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
|
@basnijholt @minrk Folks, this is my first attempt to improve the recipe. Some things worth to note:
@minrk Maybe we should decline #15 and rework the master branch, backporting the changes from this PR? |
dalcinl
force-pushed
the
ptscotch-update
branch
2 times, most recently
from
8e36be1
to
4fb4d67
Compare
- Single Makefile.inc (requires GNU Make). - Do not define `SCOTCH_PTHREAD`. Number of threads is a compile-time setting and defaults to one. Client code is forced to call `MPI_Init_thread()` requesting `MPI_THREAD_MULTIPLE` support. - Build/check/install/test scotch, then ptscotch. This should simplify a future split in two, with ptscotch depending on scotch and providing only the parallel headers and libraries. - Add trivial patch for a missing function prototype. - Add pthread patch from https://bitbucket.org/petsc/pkg-scotch. - Bump build number.
dalcinl
force-pushed
the
ptscotch-update
branch
from
4fb4d67
to
5752f22
Compare
There was a problem hiding this comment.
I think we can use the exact same Makefile and build.sh in both. I don't think that's hard. I don't really understand making ptscotch a separate package that depends on scotch when that's not the case - they are independent builds of the same package, aren't they? They certainly don't depend on each other.
Look at the commit message about the rationale of NOT defining SCOTCH_PTHREAD
Indeed, and that's why only scotch and not ptscotch defined SCOTCH_PTHREAD in this repo already.
recipe/Makefile.inc
Outdated
| CFLAGS += -DCOMMON_PTHREAD | ||
| #CFLAGS += -DSCOTCH_PTHREAD | ||
| ifeq ($(UNAME_S),Darwin) | ||
| CFLAGS += -DCOMMON_PTHREAD_BARRIER |
There was a problem hiding this comment.
Seems like it would be easier to set CFLAGS, etc. in build.sh instead of Makefile, and just have CFLAGS = ${CFLAGS} here. Makefiles aren't very good at logic.
|
#15 is updated to have a single build.sh and Makefile that works for both scotch and ptscotch. By putting the flag logic entirely in build.sh it's much simpler than a complicated Makefile. |
|
@minrk Look at the file
I'll update this PR following these facts. |
dalcinl
force-pushed
the
ptscotch-update
branch
from
5f28d4a
to
449c4c9
Compare
* `meta.yaml` can build scotch or ptscotch by just setting `name`. * `build.sh` installs either scotch or ptscotch headers and libraries. * ptscotch now (runtime) depends on scotch.
dalcinl
force-pushed
the
ptscotch-update
branch
from
449c4c9
to
ee8f19b
Compare
|
@minrk I'm done with this PR, however I'll not merge it yet. May I suggest that you decline your previous PRs and submit a new one to start fresh? I'll eventually do the same with this one. Now you can grab the recipe directory and copy it verbatim to the master branch, next change the name to Do you know any git trick we can use (maybe subtree merges) so that we can keep the recipe alone in a separate branch without all the other files, and easily merge changes to the master and ptscotch branches? PS: If you are short of time, give me access to your feedstock clone, I can start updating the master branch, or even add there your previous changes. |
If ptscotch didn't depend on scotch, we could use the conda-forge build matrix to build both packages. Unfortunately, if we want ptscotch to build against scotch changed in the same PR, then that's not going to work due to parallel builds. Other than that, I don't think anything is going to be simpler than keeping files in sync. This isn't a particularly active repo, so issuing PRs against two branches shouldn't be a huge burden. As long as we keep them in sync, applying patches twice isn't so different from having to merge one PR and then open another changing the submodule hash in the other branch. |
No description provided.