ORNL-CEES / DataTransferKit

A library for multiphysics solution transfer. ARCHIVED
https://datatransferkit.readthedocs.io/en/dtk-3.0/
BSD 3-Clause "New" or "Revised" License
47 stars 26 forks source link

Compilation error when building DTK as part of Trilinos with DTK 3.1 #565

Closed ikalash closed 4 years ago

ikalash commented 4 years ago

When I try to build DTK versions newer than 3.0 as a part of Trilinos, I get the following configuration error:

-- DTKData NOT found
DTK Revision = 'edfa050cd46e2274ab0a0b7558caca0079c2e4ca'
CMake Error: File /home/ikalash/dtkBuilds/Trilinos/DataTransferKit/packages/Search/src/ArborX_Config.hpp.in does not exist.
CMake Error at DataTransferKit/packages/CMakeLists.txt:5 (CONFIGURE_FILE):
  CONFIGURE_FILE Problem configuring file

Can you please look into this?

Rombur commented 4 years ago

@ikalash When you cloned DTK, did you get the submodule using git clone --recursive? The file that is not found is from a new submodule in DTK. Can you check that there are files in /home/ikalash/dtkBuilds/Trilinos/DataTransferKit/packages/Search/

ikalash commented 4 years ago

@Rombur : sorry for the delay in my response - I was at a workshop last week and did not get a chance to look into this until now. You are right, I was missing the ArborX and DTKData repos. I did not do git clone --recursive, and it did not occur to me that other repos would be needed, as they are not needed in the version of DTK I've been using until now (v2). Thanks for your help!

ikalash commented 4 years ago

I closed this too soon... seems I'm getting compilation errors now with branch dtk-3.1 due to missing headers:

[ 74%] Built target dtk_hybridtransport
/home/ikalash/dtkBuilds/Trilinos/build-dtk-3.0/DataTransferKit/packages/Search/src/DTK_LinearBVH_SERIAL.cpp:36:10: fatal error: DTK_LinearBVH_decl.hpp: No such file or directory
   36 | #include "DTK_LinearBVH_decl.hpp"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

Is yet another repo required to get the missing headers (e.g. DTK_LinearBVH_decl.hpp)? I cloned the code as follows:

git clone --recursive git@github.com:ORNL-CEES/DataTransferKit.git
cd DataTransferKit
git checkout dtk-3.1
ikalash commented 4 years ago

Seems this is not an issue with master, just v3.1.

Rombur commented 4 years ago

Is yet another repo required to get the missing headers (e.g. DTK_LinearBVH_decl.hpp)?

No this should be fined. I am surprised you have problems because DTK-3.1 is used in spack. Can you show us what's your configuration?

ikalash commented 4 years ago

I build DTK as a part of Trilinos, so I think the usage is slightly different than what you're used to. Here is my configure script for building DTK using cmake as a part of Trilinos: https://github.com/SNLComputation/Albany/blob/master/doc/nightlyTestHarness/do-cmake-trilinos-mpi-camobap .

I may be OK using master branch if I can get the Albany code to build/run with it. I was thinking of using a release in case master is not stable (not sure how stable it is these days - awhile ago, it wasn't stable, which is the reason we've stuck to DTK v2.0).

One "feature request" I might have regarding git clone --recursive is if it's possible to switch the paths to get the repos for ArborX and DTKData using SSH rather than HTTPS. Sandia's network prevents the clones via HTTPS, so I had to either clone on a non-Sandia machine and copy the files over, or clone the ArborX and DTKData repos separately using SSH and place them in the right place within DTK. This is not a big deal for me, though I could see it may have issues if you e.g. change the directory structure, or add additional dependencies.

Rombur commented 4 years ago

I build DTK as a part of Trilinos, so I think the usage is slightly different than what you're used to.

That's the only way to build DTK right now.

I may be OK using master branch if I can get the Albany code to build/run with it.

We haven't implemented any of the adapters that were present in DTK v2.0 Won't that be a problem for you?

One "feature request" I might have regarding git clone

I'll look into it.

aprokop commented 4 years ago

One "feature request" I might have regarding git clone --recursive is if it's possible to switch the paths to get the repos for ArborX and DTKData using SSH rather than HTTPS.

@ikalash Have you tried automatic URL rewriting (as explained here, for example)?

Rombur commented 4 years ago

This error makes no sense to me. I think that there was a problem when you pulled the submodule. The header DTK_LinearBVH_decl.hpp used to exist but that functionality was moved into ArborX. If you look at the 3.1 branch. You can see that the Search package points to ArborX and the name of the file is ArborX_LinearBVH.hpp. You get an error from a file (DataTransferKit/packages/Search/src/DTK_LinearBVH_SERIAL.cpp) that should not exist. Can you post the output of ls in DataTransferKit/packages/Search/src/

Rombur commented 4 years ago

@aprokop We can easily fix that problem https://stackoverflow.com/questions/40841882/automatically-access-git-submodules-via-ssh-or-https/44630028

aprokop commented 4 years ago

@Rombur We should certainly do that. I like the automatic URL rewriting too, though, as if it works, it would fix this problem for all other projects that one may use, instead of asking developers to fix them.

ikalash commented 4 years ago

We haven't implemented any of the adapters that were present in DTK v2.0 Won't that be a problem for you?

So looking at this more in the context of the Albany code: this may be a problem for us, yes. We were using the STK adapter. You will find what routines from DTK we were using here:

https://github.com/SNLComputation/Albany/blob/master/src/LCM/utils/dtk_interp_and_error/dtk_interp_and_error.cpp https://github.com/SNLComputation/Albany/blob/master/src/LCM/utils/dtk_interp_and_error/interpolation_volume_to_ns.cpp https://github.com/SNLComputation/Albany/blob/master/src/LCM/evaluators/bc/StrongSchwarzBC_Def.hpp https://github.com/SNLComputation/Albany/blob/master/src/LCM/evaluators/bc/SchwarzBC_Def.hpp

The first two files are stand-alone drivers that can be compiled outside of Albany and test the capabilities that we need in Albany. It appears most of the routines we were using are gone (e.g. DataTransferKit::MapOperator, all the STK adapters, etc.). I looked in the trunk version of DTK and it seems the closest example to look at involves the MapFactory, but I'm not sure how to easily convert our code w/o the STK adapters, since it really relies on them. Would you be able to look at the code linked above and give me your thoughts on whether it's possible to rewrite is using the current version of DTK?

ikalash commented 4 years ago

This error makes no sense to me. I think that there was a problem when you pulled the submodule. The header DTK_LinearBVH_decl.hpp used to exist but that functionality was moved into ArborX. If you look at the 3.1 branch. You can see that the Search package points to ArborX and the name of the file is ArborX_LinearBVH.hpp. You get an error from a file (DataTransferKit/packages/Search/src/DTK_LinearBVH_SERIAL.cpp) that should not exist. Can you post the output of ls in DataTransferKit/packages/Search/src/

You're right, the error doesn't make sense. As a sanity check, I wiped my entire build directory and re-compiled - the error went away. Previously I had only wiped out the CMake* files in my build directory which is usually sufficient but for some reason it was not the case here.

Please see my above question regarding STK adapters and migrating a code that uses dtk-2.0 and the STK adapters to a newer version of DTK. I can open a separate issue since that discussion doesn't fit that well here, if you'd prefer.

Rombur commented 4 years ago

DTK 3 does not support L2 projection but it looks like you are only using Interpolation. I think that it's possible to move to the current version of DTK but it will require quite a bit of work. If it's something you want, we should probably work with you to make it happen.

May I ask why do you want to switch to DTK 3?

ikalash commented 4 years ago

@Rombur : it's reassuring to know that it is possible to get the code working with DTK 3. I guess we would need to convert all the data that we obtain from STK currently into some alternate data structures that DTK works with now that the STK adapters are gone - is that more or less correct?

Let me check with some of my colleagues on whether it's worth doing this and get back to you.

The main reason I am looking at upgrading DTK is because on a number of occasions, changes made to Trilinos are not playing nicely with DTK v2. Most recently, DTK requires using an unsigned long Global Ordinal type: -D Tpetra_INST_INT_UNSIGNED_LONG:BOOL=ON. Changes to Tpetra/Teko are making it impossible to build those packages with that data type, however. I'm working on a PR in Trilinos to circumvent this and it is doable; but I worry that changes will keep happening to Trilinos and at some point it will be impossible to get DTK v2 to compile with Trilinos VOTD (we need to use VOTD Trilinos for our purposes). This is why I decided to look into upgrading our DTK.

Related to the issue "du jour" with the GO type described above, would it be difficult to modify DTK v2 to work with just an int or long or long long GO type (not unsigned)?

Rombur commented 4 years ago

I guess we would need to convert all the data that we obtain from STK currently into some alternate data structures that DTK works with now that the STK adapters are gone - is that more or less correct?

Yes, most of the work will be to extract the data from STK but there is also a few other changes in the interfaces.

Related to the issue "du jour" with the GO type described above, would it be difficult to modify DTK v2 to work with just an int or long or long long GO type (not unsigned)?

I think that you should just change this line but since we have never tested the code with other types I am not sure it will work.

ikalash commented 4 years ago

I think that you should just change this line but since we have never tested the code with other types I am not sure it will work.

You were close - I also needed to change the EntityId and SupportId in packages/Interface/src/Client/DTK_Types.hpp - otherwise there were link errors. I was able to get DTK to build with GO = long long w/o any changes to Trilinos, which serves my purposes for now. I make take you up on upgrading our code to a newer DTK sometime in the future, but not right now, I think. Thanks for your help!