Allow enabled of Intrepid(1) and Percept in ATDM Trilinos builds

[bartlettroscoe]

CC: @bathmatt, @rppawlo, @fryeguy52

Description

The Percept package developed in Sierra.base was recently snapshotted to the Trilinos 'develop' branch. However, this is not enabled in ATDM Trilinos builds because it depends on the Intrepid(1) package (not Intrepid2) that was disabled long ago in ATDM Trilinos builds (because the ATDM APPs were not using Intrepid(1)).

This story is to scope out the issues with allowing enable the Intrepid(1) and Percept packages and determine the level of automated testing in Trilinos.

Tasks

1. Determine if and where to test Intrepid(1) and Percept in ATDM Trilinos builds.
2. Create branch '6017-atdm-enable-percept' and allow the enable of Percept and Intrepid(1) (but disable Intrepid(1) tests).
3. Run a few trail builds of Panzer with Percept on a few platforms not covered by Trilinos PR testing.
4. Create PR with branch '6017-atdm-enable-percept'.

[bartlettroscoe]

@rppawlo and @bathmatt,

I propose that we remove the disable of Intrepid(1) and allow the enable of Percept in all of that ATDM Trilinos builds, but we disable the native Intrepid(1) tests and just have the native Percept and Panzer tests that use Percept and Intrepid(1). (However, I don't think Percept has any native tests, at least looking at this full CI build.)

Therefore, the only concern would be the portability of the Panzer tests that use Percept. But I guess we should just allow all of these tests to get enabled and see how they run on the various ATDM Trilinos builds.

Sound good?

P.S. But if Percept is using Intrepid(1), that means that it does not run on a GPU, right?

[rppawlo]

That sounds good @bartlettroscoe !

I don't think percept executes kernels on gpus unless it calls stk functions that have been converted to kokkos. The refinement will be run on host for now.

[bathmatt]

I think that panzer tests percept as EMPIRE intends to use it

[rppawlo]

I think that panzer tests percept as EMPIRE intends to use it

yes.

[bathmatt]

@bartlettroscoe let me know when you have a PR ready to test, I'll verify it works...

[rppawlo]

@bartlettroscoe - I believe you will need the following PR to enable percept builds:

#6018

A push to stk last week resulted in stk and percept going out of sync. In the future stk and percept will be pushed together to stay in sync.

@bricar
@alanw0
@bathmatt

[bartlettroscoe]

@rppawlo said:

A push to stk last week resulted in stk and percept going out of sync.

How did this pass the Trilinos PR builds? There there not tests in Panzer that would break if STK and Percept are not consistent in a PR branch?

[rppawlo]

Maybe it was just fixes to deprecated code that hasn't been removed from stk yet?

[bartlettroscoe]

@rppawlo said:

@bartlettroscoe - I believe you will need the following PR to enable percept builds: #6018

Looks like that PR has build errors (warnings to errors in gcc-7.2.0 and code that just does not build with gcc-4.8.4).

I can create a branch that enables Intrepid(1) and Percept with Panzer but I guess it will be of no use to EMPIRE until the issues in #6018 are resolved and the PR is merged, right?

[alanw0]

@bartlettroscoe @rppawlo Sorry for the late reply, but regarding the question about about stk and percept getting out of sync: these are changes that had happened in the sierra repo, where a new stk api was being used by percept. The stk change hadn't made it to the trilinos repo yet. That's why percept couldn't be pushed to trilnios at that point. stk is now up-to-date in trilinos, so it should be possible to push percept up to trilinos also.

[bartlettroscoe]

@alanw0 said:

these are changes that had happened in the sierra repo, where a new stk api was being used by percept. The stk change hadn't made it to the trilinos repo yet. That's why percept couldn't be pushed to trilnios at that point. stk is now up-to-date in trilinos, so it should be possible to push percept up to trilinos also.

But that means that Trilinos 'develop' has new STK (with the new API) and the old Percept. But should that not mean that Percept would be broken? Or was the new STK API backward-compatible so you could use a new STK with an old Percept? Just trying to understand how could have passed the Trilinos PR builds.

[alanw0]

@bartlettroscoe Yes the new stk api was a new function, and the other function that percept had been using is still in place. Thus an older percept could still build/run with the newer stk.

[bartlettroscoe]

CC: @rppawlo

@bathmatt,

FYI: The Percept update in PR #6018 has not been merged to the Trilinos 'develop' branch yet. It is still showing a build error in the gcc-7.2.0 PR build shown here (due to a warning elevated to an error). And that was way back on 10/2/2019. There has not been any action on that PR since then.

Therefore, until PR #6018 gets merged, is there any point in enabling Percept the ATDM Trilinos builds? That is why I have not moved on this issue #6017 yet. (I am subscribed to PR #6018 so when it gets merged, I will know it.)

[rppawlo]

@bartlettroscoe - yes - you can enable now without PR #6018 . Those are changes to percept for deprecated code in stk. The merge is on hold a little longer as Brian is putting up more changes to remove boost from percept. If you could enable this it would be a big help as I can go ahead with more empire testing.

[bricar]

I just rebased and pushed my 2 commits to bricar/Trilinos branch "percept-sync-from-sierra". I believe I have the build issues resolved but am building gcc 4.8.4 and 7.2.0 to confirm. I am not too familiar with the Trilinos PR process so may need some help there.

The Intrepid issue discussed above was also new to me. If you think I should try to move Percept to Intrepid2 let me know, though I don't know the level of effort it would take.

I am confident that Percept does not directly depend on Boost except through the TPLs STKUtil and STKTransfer.

[rppawlo]

@bricar - that's excellent news on the boost dependency! Thanks! Alan is working to remove boost completely from stk and will allow us to drop the dependency completely from Trilinos.

Moving to intrepid2 would allow us to build less code. The only complexity is that it is the kokkos version of intrepid, so the evaluation kernels look a little different - functions use kokkos view semantics and sometimes require scratch memory on device. Is percept slated to be run on device in the future? If so, converting to intrepid2 would probably be needed anyway. Early on intrepid 1 and 2 could not be built together since they shared the same namespace and had collisions. Now they have separate namespaces, they can be built together. So it is not pressing other than increasing build times for atdm testing and modifying build scripts. In the long term, the apps would prefer to lessen the number of packages they depend on.

[alanw0]

We are eager to remove boost from stk, but it's a journey... hopefully soon.

[bartlettroscoe]

Working on this now.

@rppawlo, what new tests should get enabled when Percept support is enabled in Panzer? The only test that I can find is:

#TRIBITS_ADD_EXECUTABLE_AND_TEST(
#  tUniformRef
#  SOURCES tUniformRef.cpp ${UNIT_TEST_DRIVER}
#  NUM_MPI_PROCS 2
#  COMM serial mpi
#  )

in the file packages/panzer/adapters-stk/test/stk_interface_test/CMakeLists.txt but that test is commented out.

Is there no testing of Panzer support for Percept?

[rppawlo]

It's in the file:

./panzer/adapters-stk/test/stk_interface_test/tExodusReaderFactory.cpp

When percept is enabled, an extra test is added in this file.

[bartlettroscoe]

It's in the file:

./panzer/adapters-stk/test/stk_interface_test/tExodusReaderFactory.cpp

When percept is enabled, an extra test is added in this file.

Okay, got it. It is a single unit test:

#ifdef PANZER_HAVE_PERCEPT
TEUCHOS_UNIT_TEST(tExodusReaderFactory, percept)
{
  ...
}
#endif

And we can see this is getting run in the Trilinos PR testing (and a couple other builds) by running the query:

• https://testing.sandia.gov/cdash/queryTests.php?project=Trilinos&begin=2019-10-10&end=2019-10-15&filtercount=2&showfilters=1&filtercombine=and&field1=testname&compare1=63&value1=ExodusReaderFactory&field2=testoutput&compare2=95&value2=ExodusReaderFactory_percept

It shows it is getting run in all of the PR builds including the CUDA build on 'ride'. (NOTE: Click the "Show Matching Output" text on the upper right to see that unit test running and passing.)

Therefore, I think this is pretty well protected in PR testing. That means that I will need to do less testing. (Just a single build on 'waterman' should do just to be safe.)

[bartlettroscoe]

@rppawlo and @bathmatt,

PR is #6105. Please approve (and note the minimal changes and where the changes were made).

[bartlettroscoe]

@rppawlo and @bathmatt,

PR #6105 just merged to Trilinos github 'develop'.

[rppawlo]

Thanks @bartlettroscoe !

[bartlettroscoe]

This has been done for over a week.

The only negative impact was the enable of some new ROL tests that caused failures. See #6124. If something other is broken, we will address that in new issues.

Closing this issue as complete.