sandialabs / Albany

Sandia National Laboratories' Albany multiphysics code
Other
272 stars 90 forks source link

Remove unused/unsupported physics, provide legacy code last working git tag #359

Closed bartgol closed 5 years ago

bartgol commented 6 years ago

This issue is to decide what packages are to be discontinued in Albany. By that I mean packages/physics that are no longer maintained, and that have become a maintenance burden, especially when Albany undergoes refactoring efforts.

Such packages should be removed from the current master (together with all their configurations options/macros that are specific to them). We should provide an Albany tag (together with a working trilinos tag) where such packages are still present and functional (= test passing).

Of course, if some user is willing to port such packages to the current master framework, they are more then welcome. We only do this because of the lack of manpower (and funding money) to maintain packages.

Here is a list of the current packages/physics/subfolders that are non-core. Please, add a comment below if you want the package to remain fully supported in the master branch. By requesting such maintenance, you should also commit some of your time to perform such maintenance. We cannot afford to keep packages that have no manpower to maintain them. For such packages, we can only provide a working git tag. I will update this message adding a checkmark next to the package, and one or more names for people responsible for maintaining it.

I would like to get a response by next week, so that we can take a decision at the next Albany conference call.

bartgol commented 6 years ago

I should add that if a package has no issues or can be maintained with minimal effort, we should try to keep it in master. I am not trying to discontinue all packages that are not working but could with just a few minutes of coding. We are still trying to make an effort to keep Albany a truly multiphysics library. However, there are some efforts ongoing that are already requiring a lot of time, and for which it would take considerable effort to maintain all packages.

ikalash commented 6 years ago

Should we tag the last known owners of some of the packages to bring this issue to their attention and have them comment?

bartgol commented 6 years ago

Good idea. Tagging people listed in CODEOWNERS: @gahansen , @tjcorona , @ibaned , @bgranzow , @lxmota , @mperego .

ikalash commented 6 years ago

Also: @enielse (QCAD), @jrobbin (ATO), @plindsa and @jfike (MOR).

bartgol commented 6 years ago

@ikalash You may also want to update CODEOWNERS, cause it lists you as owner of those packages.

ikalash commented 6 years ago

Where is CODEOWNERS? Is it in a file in the repo? I would not consider myself the owner of these packages...

bartgol commented 6 years ago

Yes, in the top folder. Apparently github parses it, and uses it also to notify code owners when a PR touches paths that are under their responsibility. Makes it easier (actually, automatic) for people without write access to the repo to tag the proper code maintainer.

bgranzow commented 6 years ago

As far as I know, the CTM package has no plans for further use and depends on a version of Simmetrix that I no longer have access to (meaning I can't really maintain it). I would be fine with removing it. It might make sense for me to ask if anyone at RPI cares about this at the next Albany telecon.

ibaned commented 6 years ago

@maxrpi is okay with leaving CTM as a tag and not updating it.

jrobbin commented 6 years ago

I'll commit to maintain the ATO content. There's a project starting in FY19 that will leverage this capability.

ikalash commented 6 years ago

We should also probably add Peridigm and SCOREC to the list of packages. There have been failing tests for these for awhile (see issue #360).

bartgol commented 6 years ago

Done!

So far there's a lot of unchecked packages. Since there was no telecon on Labor Day, I think I will wait till the next telecon before we make any final decision.

ikalash commented 6 years ago

@djlittl : can you please comment on this issue regarding plans for supporting peridigm? @ibaned , @bgranzow : are you guys the owners of SCOREC still? If not, can you please tag the current owner so they can comment on this issue.

ikalash commented 6 years ago

Another related question: is anyone interested in AlbanyDakota and AlbanyAnalysis? If not perhaps these should be removed too. I am the only one testing these nightly, it looks like.

lxmota commented 6 years ago

Not as far as LCM is concerned.

bartgol commented 6 years ago

I think we need AlbanyAnalysis for optimiztion, no?

mperego commented 6 years ago

Yes we need AlbanyAnalysis for optimization. It is also used in LCM Peridigm.

ikalash commented 6 years ago

Oh right. So, for optimization and peridigm you use Dakota via AlbanyAnalysis? Somehow I didn't realize that. I was referring to the examples that have "Analysis" in the name. I upgraded the OS on my machine and now there's some issue with the tests that use Dakota. It may have to do with me having too old of a version of Dakota to work with gcc/8.1.1 which is very new. I thought if the tests aren't needed I can just disable them, hence this question.

mperego commented 6 years ago

We don't use Dakota for the optimization for LandIce and Peridigm, we use ROL.

ikalash commented 6 years ago

Ok, this is what I thought. Let me rephrase my question: is the use of Dakota by AlbanyDakota/AlbanyAnalysis obsolete? I suspect the answer is yes, but it would be good to have someone second that. Maybe we can add it to the checklist at the top of this issue.

bartgol commented 6 years ago

I thought John was using Albany and Dakota for the UQ stuff in Prospect

mperego commented 6 years ago

No that's a myth!

Sent from VMware Boxer

On Sep 7, 2018 8:12 AM, Luca Bertagna notifications@github.com wrote:

I thought John was using Albany and Dakota for the UQ stuff in Prospect

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/gahansen/Albany/issues/359#issuecomment-419452160, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AFqlZCow8QJBXCo70_olXZLlwfADD-DMks5uYn7MgaJpZM4WOdaP.

ikalash commented 6 years ago

We never used embedded UQ capabilities in Albany for PISCEES/ProSPect. We used to wrap DAKOTA around Albany, but that was years ago; now we have a different approach for UQ in ProSPect that doesn't use DAKOTA at all.

bartgol commented 6 years ago

Is there a test in one of the nightly builds that tests Albany-Dakota interfaces? If not, then I also wonder if it's ok to keep it around. It doesn't bother much, since it's basically two files, with little to maintain (or nothing at all if you don't/can't have Dakota on your machine), so it's not the same as maintaining packages in Albany. It is more a question of "can we guarantee it works, if someone tomorrow downloads Albany and uses it with Dakota?".

ikalash commented 6 years ago

There were a couple of tests that only I was running. I upgraded the OS on my machine and they seg fault now. It may have to do with me using an old version of Dakota. I am wondering if maintaining the tests is worthwhile if no one is using them.

agsalin commented 6 years ago

AlbanyDakota was an earlier interface to Dakota. Dakota is also available as an option under AlbanyAnalysis via a Piro interface to Dakota. I had also kept the direct Dakota interface around since it had the cool capability of having Dakota select MPI_Comm for Albany to use, and it could spawn numerous Albany runs in parallel function evaluations on sub-communicators. That's the history/nostalgia -- No reason to keep AlbanyDakota anymore or the two files.

ikalash commented 6 years ago

Thanks for the info, @agsalin. I'm fine keeping things around even if they're not used provided it's fairly straightforward to do so. I can look at the Dakota issue when I have more time in a few weeks, which would likely result in opening an issue for the Dakota team to look at b/c I don't think the problem is in Albany from the backtrace. I'm not sure if that'll be helpful to the Dakota team or a waste of their time - this is why I started the discussion.

ibaned commented 6 years ago

I don't know... I'd think twice about spending time/money maintaining something that isn't used...

ikalash commented 6 years ago

@ibaned . Right I agree. Let’s talk about it on the telecom Monday. Hopefully we can make some definite decisions about what stays and what goes.

djlittl commented 5 years ago

I am using the code for coupling with Peridigm for a paper that I'm working on, and it's also been cited on a few SBIR proposals over the last two years. I honestly don't know what the right path forward is. I have no funds for testing, maintenance, support, etc. It's also tricky to test, because it requires a Peridigm build. Suggestions welcome.

gahansen commented 5 years ago

I see a "strategic" benefit for some of these packages - like Peridigm. While we don't have a path to maintain this link as we would like, it involves two major Sandia capabilities and I hold out hope that interest in multiscale material behavior might make this an important feature "any day now". I guess the question is - is it harder to keep it minimally functional (the current dashboard green) or tag it and bring it back to life when the time comes? Is it possible to solicit a volunteer to keep the dashboard green on the tests of the Peridigm link?

gahansen commented 5 years ago

Tagging @tjcorona - TJ what do you see as the future of the AFRL subdirectory / capability? I moved a little of that into one of the LCM evaluators but I don't think we finished that or included a test for it.

gahansen commented 5 years ago

I am the "owner" of Hydride. It still compiles on CEE but isn't tested now. I'd suggest we tag the commit that removes this and call that good.

bartgol commented 5 years ago

There are some evaluators/responses in the main PHAL folders that have the QCAD prefix. If QCAD is eliminated, what should we do with them? Should we purge them, or move them to PHAL?

mperego commented 5 years ago

If they are used by other packages, or if they could be useful, I think it would make sense to move them to PHAL. Otherwise I'd purge them.

ikalash commented 5 years ago

I agree. I believe there are some capabilities from the QCAD namespace used in LCM (e.g., the MaterialDatabase). @lxmota would be able to say more. I'd move these to the PHAL namespace.

bartgol commented 5 years ago

Ok, I agree. On a similar note, I should try to keep stuff like Albany_ThermoElectrostaticsProblem.*pp, which is only compiled if QCAD is enabled, but does not seem to use any of the QCAD evaluators/responses. It was probably a good idea to move that pb to the main Albany anyways.

bartgol commented 5 years ago

Wait, that problem reuires QCAD_PoissonResid. I may grab that single non-phal evaluator, and port it to the pde examples.

lxmota commented 5 years ago

@ikalash No, long ago I removed that QCAD dependency from LCM. I created a MaterialDatabase with far fewer features for use in LCM. The QCAD version was overly complex for LCM.

ikalash commented 5 years ago

@lxmota : ah yes I remember now. Good, it should simplify the removal of QCAD then.

ikalash commented 5 years ago

Per our discussion this morning, I checked out Trilinos from before STK broke things (8/20), rebuilt it, built Albany on top of it, and ran my nightly test scripts. Here are the packages enabled there:

    "-DALBANY_TRILINOS_DIR:PATH=${TRILINSTALLDIR}"
    "-DENABLE_LCM:BOOL=ON"
    "-DENABLE_CONTACT:BOOL=OFF"
    "-DENABLE_LCM_SPECULATIVE:BOOL=OFF"
    "-DENABLE_HYDRIDE:BOOL=ON"
    "-DENABLE_SG:BOOL=OFF"
    "-DENABLE_ENSEMBLE:BOOL=OFF"
    "-DENABLE_LANDICE:BOOL=ON"
    "-DENABLE_TSUNAMI:BOOL=ON"
    "-DENABLE_AERAS:BOOL=ON"
    "-DENABLE_QCAD:BOOL=ON"
    "-DENABLE_MOR:BOOL=ON"
    "-DENABLE_ATO:BOOL=ON"
    "-DENABLE_ALBANY_EPETRA_EXE:BOOL=ON"
    "-DENABLE_AMP:BOOL=OFF"
    "-DENABLE_ASCR:BOOL=OFF"
    "-DENABLE_AFRL:BOOL=OFF"
    "-DENABLE_CHECK_FPE:BOOL=OFF"
    "-DENABLE_MPAS_INTERFACE:BOOL=OFF"
    "-DENABLE_CISM_INTERFACE:BOOL=ON"
    "-DENABLE_CISM_CHECK_COMPARISONS:BOOL=ON"
    "-DENABLE_CISM_EPETRA:BOOL=ON"
    "-DENABLE_CISM_REDUCED_COMM:BOOL=OFF"
    "-DSEACAS_EPU=/home/ikalash/Trilinos/seacas-build/install/bin/epu"
    "-DSEACAS_EXODIFF=/home/ikalash/Trilinos/seacas-build/install/bin/exodiff"
    "-DSEACAS_ALGEBRA=/home/ikalash/Trilinos/seacas-build/install/bin/algebra"
    "-DCISM_INCLUDE_DIR:FILEPATH=${CTEST_SOURCE_DIRECTORY}/cism-piscees/libdycore"
    "-DINSTALL_ALBANY:BOOL=ON"
    "-DCMAKE_INSTALL_PREFIX:BOOL=${CTEST_BINARY_DIRECTORY}/IKTAlbanyInstall"
    "-DENABLE_PARAMETERS_DEPEND_ON_SOLUTION:BOOL=ON"
    "-DCISM_EXE_DIR:FILEPATH=${CTEST_BINARY_DIRECTORY}/IKTCismAlbany"
    "-DENABLE_USE_CISM_FLOW_PARAMETERS:BOOL=ON"
    "-DENABLE_LAME:BOOL=OFF"

When I run ctest in a regular full build (with the above settings), unfortunately there are some LCM tests failing, namely the dynamics Schwarz tests I mentioned seem to have broken recently:

        307 - Schwarz_Alternating_Dynamics_Cubes (Failed)
    308 - Schwarz_Alternating_Dynamics_CubesInelastic (Failed)
    309 - Schwarz_Alternating_Dynamics_CubesInelastic_Parallel (Failed)
    310 - Schwarz_Alternating_Dynamics_Clamped_Parallel (Failed)
    311 - Schwarz_Alternating_Dynamics_Torsion_Parallel (Failed)

The error looks like this: http://cdash.sandia.gov/CDash-2-3-0/testDetails.php?test=3938397&build=75966 . It looks like this may have been a problem introduced by a merge of a PR by @bartgol looking at past github issues, namely #350. These were just showing up on proxima, but now they show up on my machine. I bet that's because I upgraded my OS to Fedora 28 and gcc/8.1.1, which are now consistent with proxima.

The question is do we want to wait till these tests are fixed before moving things to a tag or not? If one does not build Trilinos with Tempus turned on, the tests will not run (and hence will not fail).

There are also 4 failing tests in an OpenMP build:

       141 - MechanicsTensileCT (Failed)
        230 - CrystalPlasticity_OrientationOnMesh (Failed)
        231 - CrystalPlasticity_OrientationOnMesh_np4 (Failed)
        243 - CrystalPlasticity_SchwarzBar_modified_np1 (Failed)

This seems to be another existing known issue, namely #339. As I said this morning, I would not be too concerned about the OpenMP tests since I don't think anyone using the obsolete physics moved to a tag will be running with OpenMP (nor should they, since most of the physics are Epetra-based).

Here is the Trilinos hash that produced the results described above: 22c79ca9434ce101fc4c4200a90c5d9e42692ca5 . BTW, this is develop Trilinos. I am not sure if you were planning to give SHAs for develop or master Trilinos @bartgol . I think it doesn't matter as long as things are clean with the SHA.

ibaned commented 5 years ago

I think we agreed that since LCM will continue to be supported, we don't need LCM's tests to pass in order to approve tags for other sub-projects, as long as all of their sub-project tests pass.

ikalash commented 5 years ago

One thing that came to mind while writing the reply above. Maybe we should mention the compilers used to create the working builds in the READMEs? It can happen that things start failing with newer compilers (as evidenced by the LCM Schwarz tests). So it's conceivable that something might break with time as compilers are upgraded even though the tag was working perfectly at the time it was created.

bartgol commented 5 years ago

This was completed by PR #363 .

@ikalash I did not specify working compilers in the readme's or in the tags. I will edit them, by looking at the compilers currently used on cdash.

ikalash commented 5 years ago

@bartgol sounds good. The compiler I use is: gcc/8.1.1 .