Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

embedding the uwuw preprocessing in the plugin #62

Draft
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

bam241
Copy link
Member

@bam241 bam241 commented Oct 6, 2020

this should allow to preprocess material and tallies directly from Trelis.

this is compiling but has not being tested.

Feel free to comment even if this is not finished yet!

@ljacobson64
Copy link
Member

Note that this PR depends on svalinn/DAGMC#703

I was able to build a working version of the plugin after adding these lines to the "Create the Tarball" section of the README:

cp -pPv ${HOME}/plugin-build/DAGMC/lib/libpyne_dagmc.so .
cp -pPv ${HOME}/plugin-build/DAGMC/lib/libuwuw.so .

and

patchelf --set-rpath /opt/Trelis-16.5/bin/plugins/svalinn libpyne_dagmc.so
patchelf --set-rpath /opt/Trelis-16.5/bin/plugins/svalinn libuwuw.so

@ljacobson64
Copy link
Member

Ah, never mind. I tried running

export dagmc "test_uwuw.h5m" faceting_tolerance 1e-3 make_watertight verbose pyne_mat_lib "/opt/software_native/pyne/lib/python2.7/site-packages/pyne/nuc_data.h5"

and I got the following:

Setting faceting tolerance to 0.001
Setting length tolerance to 0
Setting normal tolerance to 5
Looking for the PyNE material Lib in /opt/software_native/pyne/lib/python2.7/site-packages/pyne/nuc_data.h5
hdf5 path set to 
Found 16 entities of dimension 0
Found 24 entities of dimension 1
Found 14 entities of dimension 2
Found 4 entities of dimension 3
***** Faceting Summary Information *****
----- All curves faceted correctly  -----
----- All surfaces faceted correctly  -----
***** End of Faceting Summary Information *****
trelis: symbol lookup error: /opt/Trelis-16.5/bin/plugins/libsvalinn_plugin.so: undefined symbol: _ZN17uwuw_preprocessorC1ESsSsSsSsbb

which is the same issue you're seeing

@ljacobson64
Copy link
Member

You know, I bet this is caused by the _GLIBCXX_USE_CXX11_ABI=0 thing...

@gonuke do you know why we need this flag? It's been there since the beginning and all I know about it is that the plugin won't work without it.

@bam241
Copy link
Member Author

bam241 commented Oct 7, 2020

I tried without, did not change...

@ljacobson64
Copy link
Member

If you remove the _GLIBCXX_USE_CXX11_ABI=0 then it just segfaults

-- DAGMC export command available.
./test.sh: line 3:   947 Segmentation fault      (core dumped) trelis -batch -nographics -nojournal test.jou

@bam241
Copy link
Member Author

bam241 commented Oct 7, 2020

missing symbol is:
undefined symbol: _ZN17uwuw_preprocessorC1ESsSsSsSsbb
but symbol in libuwuw are:

nm -g libuwuw.so |grep uwuw_prepro
000000000000d480 T _ZN17uwuw_preprocessor13print_summaryEv
00000000000141f0 T _ZN17uwuw_preprocessor15process_talliesEv
000000000000d2a0 T _ZN17uwuw_preprocessor15property_vectorERKSt6vectorIiSaIiEE
000000000000dad0 T _ZN17uwuw_preprocessor17check_tally_propsENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES5_ii
0000000000017e70 T _ZN17uwuw_preprocessor17process_materialsEv
0000000000016dd0 T _ZN17uwuw_preprocessor18write_uwuw_talliesEv
0000000000013800 T _ZN17uwuw_preprocessor19create_new_materialEN4pyne8MaterialENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
00000000000129a0 T _ZN17uwuw_preprocessor20check_material_propsESt6vectorINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESaIS6_EES8_i
000000000000e130 T _ZN17uwuw_preprocessor20make_tally_groupnameENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEim
00000000000132f0 T _ZN17uwuw_preprocessor20write_uwuw_materialsEv
0000000000011630 T _ZN17uwuw_preprocessorC1ENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES5_S5_S5_bb
0000000000011630 T _ZN17uwuw_preprocessorC2ENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES5_S5_S5_bb
0000000000011f00 T _ZN17uwuw_preprocessorD1Ev
0000000000011f00 T _ZN17uwuw_preprocessorD2E

I am wondering is the difference are important there :
_ZN17uwuw_preprocessorC1ESsSsSsSsbb vs _ZN17uwuw_preprocessorC1ENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES5_S5_S5_bb

@ljacobson64
Copy link
Member

I was right! I re-built moab and DAGMC with -D_GLIBCXX_USE_CXX11_ABI=0 enabled, and then rebuilt the plugin. This time this is what I got:

Setting faceting tolerance to 0.001
Setting length tolerance to 0
Setting normal tolerance to 5
Looking for the PyNE material Lib in /opt/software_native/pyne/lib/python2.7/site-packages/pyne/nuc_data.h5
hdf5 path set to 
Found 16 entities of dimension 0
Found 24 entities of dimension 1
Found 14 entities of dimension 2
Found 4 entities of dimension 3
***** Faceting Summary Information *****
----- All curves faceted correctly  -----
----- All surfaces faceted correctly  -----
***** End of Faceting Summary Information *****
Loading file test_uwuw.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.001
No material property found for volume with ID 6
Every volume must have exactly one mat: property

@ljacobson64
Copy link
Member

Building moab and DAGMC with -D_GLIBCXX_USE_CXX11_ABI=0 changes the symbols in libuwuw.so to be:

lucas@DESKTOP-ONST2GV:/opt/Trelis-16.5/bin/plugins/svalinn $ nm -g libuwuw.so  | grep uwuw_preproc
000000000000c880 T _ZN17uwuw_preprocessor13print_summaryEv
0000000000012540 T _ZN17uwuw_preprocessor15process_talliesEv
000000000000c6a0 T _ZN17uwuw_preprocessor15property_vectorERKSt6vectorIiSaIiEE
000000000000ced0 T _ZN17uwuw_preprocessor17check_tally_propsESsSsii
0000000000014ba0 T _ZN17uwuw_preprocessor17process_materialsEv
00000000000140a0 T _ZN17uwuw_preprocessor18write_uwuw_talliesEv
0000000000011cc0 T _ZN17uwuw_preprocessor19create_new_materialEN4pyne8MaterialESs
0000000000010e00 T _ZN17uwuw_preprocessor20check_material_propsESt6vectorISsSaISsEES2_i
000000000000d4a0 T _ZN17uwuw_preprocessor20make_tally_groupnameESsim
0000000000011740 T _ZN17uwuw_preprocessor20write_uwuw_materialsEv
000000000000fa80 T _ZN17uwuw_preprocessorC1ESsSsSsSsbb
000000000000fa80 T _ZN17uwuw_preprocessorC2ESsSsSsSsbb
00000000000101a0 T _ZN17uwuw_preprocessorD1Ev
00000000000101a0 T _ZN17uwuw_preprocessorD2Ev

_ZN17uwuw_preprocessorC1ESsSsSsSsbb matches the one that the plugin originally said was missing.

@bam241
Copy link
Member Author

bam241 commented Oct 7, 2020

great!!!

thx for finding the solution !

@ljacobson64
Copy link
Member

Not sure if this actually the best solution though; depends on Paul's answer to my question

@ljacobson64
Copy link
Member

See here 55a2f7c...ljacobson64:preproc for the changes I made

@pshriwise
Copy link
Member

pshriwise commented Oct 7, 2020

Sorry just catching up here. After some digging on this, I think the ABI flag setting was related to specific versions of gcc being incompatible. Specifically, binaries compiled using gcc >=5.1 would no longer work with binaries compiled using gcc <4.8.2 if this flag was not set. It had something to do with a correction in how C++11 extensions were handled. Given that these are now pretty old compiler versions, I think we can remove this setting. All of the versions of Ubuntu we're building the plugin for at this point should have binaries compiled with something newer than 5.1.

As to why it suddenly mattered w/ UWUW, this might be the first instance of us using a piece of the DAGMC API with a std::string, which is one C++ component affected by this ABI conflict? That's my initial guess anyway.

@ljacobson64
Copy link
Member

It does matter though... if you remove that flag, then Trelis segfaults as soon as it tries to load the plugin

@pshriwise
Copy link
Member

pshriwise commented Oct 7, 2020

I'll look more closely later tonight, but I think it's sort of an all-or-nothing thing (all projects need to either use or not use the flag). I'll try to elaborate later. I could be totally wrong though.

@bam241
Copy link
Member Author

bam241 commented Oct 7, 2020

As to why it suddenly mattered w/ UWUW, this might be the first instance of us using a piece of the DAGMC API with a std::string, which is one C++ component affected by this ABI conflict? That's my initial guess anyway.

yes I think that's right.

I am not sure we can avoid the flag (on the Trelis side) tho...

@pshriwise
Copy link
Member

pshriwise commented Oct 8, 2020

@bam241 good point. If the Trelis libraries are compiled with a version of GCC < 5.1 then we might have to include these flags throughout our stack when building the plugin.

Here's some info from GCC on what's going on: https://gcc.gnu.org/onlinedocs/gcc-5.2.0/libstdc++/manual/manual/using_dual_abi.html

@pshriwise
Copy link
Member

pshriwise commented Oct 8, 2020

I think Trelis 16.5 (which is what it looks like was being used to test the plugin bulid for this PR) was compiled with GCC <5.1. Here's the output of readelf -p .comment libcutbiti19.so.

String dump of section '.comment':
  [     0]  GCC: (GNU) 4.4.7 20120313 (Red Hat 4.4.7-23)
  [    2d]  GCC: (GNU) 4.9.2 20150212 (Red Hat 4.9.2-6)

For Trelis 17.1, however, it appears a newer compiler may have been used for some/all of the compilation?

String dump of section '.comment':
  [     0]  GCC: (GNU) 4.4.7 20120313 (Red Hat 4.4.7-23)
  [    2d]  GCC: (GNU) 6.3.1 20170216 (Red Hat 6.3.1-3)

So I'd be curious if this fix for setting the ABI flag is necessary for 17.1 as opposed to 16.5. OR if using it actually breaks the plugin build against 17.1 because it was created with a different compiler. OR perhaps we'll verify that we do in fact need to include this setting throughout our stack when building the plugin regardless of what Trelis version we're working with -- in which case none of this matters and we move forward with the changes @ljacobson64 has provided 😄

@ljacobson64
Copy link
Member

Thanks for all this info; very enlightening.

I have been using Trelis 16.5 for my testing, yes. I have not tried 17.1 yet.

If Trelis 17 uses a newer compiler and thus requires our entire stack to not contain the ABI flag, while Trelis 16 does require the ABI flag, then we would need to start providing 2 different versions of the plugin.

@ljacobson64
Copy link
Member

The verdict is in: the ABI flag is needed for both 16.5 and 17.1.

@pshriwise
Copy link
Member

Alrighty! Let's update the install instructions as per your commit above. Good to know we can have a consistent approach to this for Trelis 16 and 17.

Thanks for verifying this, @ljacobson64!

@bam241
Copy link
Member Author

bam241 commented Oct 9, 2020

@ljacobson64 how did you pass the ABI flag in MOAB/DAGMC installs ? (I can include this fix in the PR as it is related to compiling and running this upgrade of the Plugin...)

@pshriwise
Copy link
Member

Looking at the commit he referenced above, it seems he set the CXXFLAGS environment variable when running CMake for MOAB & DAGMC.

@bam241
Copy link
Member Author

bam241 commented Oct 9, 2020

thx I missed the link part. I'll include those commit in the PR :)

@ljacobson64 ljacobson64 mentioned this pull request Oct 9, 2020
pshriwise
pshriwise previously approved these changes Oct 20, 2020
Copy link
Member

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I can't try this out, but everything looks good to me!

@bam241 bam241 marked this pull request as draft October 20, 2020 13:08
@bam241
Copy link
Member Author

bam241 commented Oct 20, 2020

converting to Draft, I still want to change the command line side.

We were thinking about: setting up a variable for the pyne MaterialLibrary and the datapath and just a boolean in the export to dagmc command, to simplify usage.

@pshriwise
Copy link
Member

Sounds good. Let us know when it's ready for review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants