search

nasa / trick

Trick Simulation Environment. Trick provides a common set of simulation capabilities and utilities to build simulations automatically.
Other
32 stars 16 forks source link

MonteCarloGenerate: Add more metadata to MonteCarlo_Meta_data_output #1608

Closed ddj116 closed 9 months ago

ddj116 commented 10 months ago

Originally requested by a user in discussions in Ramtares GitLab merge request 6053, this MR implements a better and more robust solution than the originally proposed approach.

coveralls commented 10 months ago

Coverage Status

coverage: 55.85% (+0.1%) from 55.744% when pulling bbf48ba0b54c2f2ae21a6801a85884ffa20d34ba on 1574-more-mc-metadata into 9ead0a28be7c9daff918319d0929c854e2a4abbd on master.

ddj116 commented 10 months ago

FYI @sharmeye @hchen99 @jmpenn I believe this is ready for review. I've got some intermediate commits on the branch, one of which was producing a segfault in test/SIM_mc_generation, but it was so finnicky the segfault would disappear when building Trick with -g -O0, putting me in a chicken/egg situation where I couldn't debug the problem unless I removed the symbols which let me inspect the problem in the first place 🐔 🥚 . The tip of the branch is stable in both debug and non-debug builds of Trick. Suggest squashing commits when y'all are ready. Let me know if you want to discuss the details of the mysterious disappearing segfault.

hchen99 commented 10 months ago

FYI @sharmeye @hchen99 @jmpenn I believe this is ready for review. I've got some intermediate commits on the branch, one of which was producing a segfault in test/SIM_mc_generation, but it was so finnicky the segfault would disappear when building Trick with -g -O0, putting me in a chicken/egg situation where I couldn't debug the problem unless I removed the symbols which let me inspect the problem in the first place 🐔 🥚 . The tip of the branch is stable in both debug and non-debug builds of Trick. Suggest squashing commits when y'all are ready. Let me know if you want to discuss the details of the mysterious disappearing segfault.

Thanks, Dan! We'll review.

ddj116 commented 9 months ago

@hchen99 @jmpenn @sharmeye I can't make any sense of the MacOS failure to build Trick - something about errors coming from swig_int_templates.hh . A few CI pipelines back this same code (with a few minor differences) built fine on Mac. Could this be an non-determinism CI issue or do you think it's a real problem we need to track down?

hchen99 commented 9 months ago

@hchen99 @jmpenn @sharmeye I can't make any sense of the MacOS failure to build Trick - something about errors coming from swig_int_templates.hh . A few CI pipelines back this same code (with a few minor differences) built fine on Mac. Could this be an non-determinism CI issue or do you think it's a real problem we need to track down?

I also checked out this branch and having no problem building trick and all tests on my Mac. I am gonna to give it a try in a container to see if anything needs updating for Mac workflow. Btw, thank y'all for addressing review comments.

hchen99 commented 9 months ago

@hchen99 @jmpenn @sharmeye I can't make any sense of the MacOS failure to build Trick - something about errors coming from swig_int_templates.hh . A few CI pipelines back this same code (with a few minor differences) built fine on Mac. Could this be an non-determinism CI issue or do you think it's a real problem we need to track down?

I also checked out this branch and having no problem building trick and all tests on my Mac. I am gonna to give it a try in a container to see if anything needs updating for Mac workflow. Btw, thank y'all for addressing review comments.

Ok, the latest github macos 12 runner image had an update recently that had python updated from 3.11 to 3.12. And looks like Python 3.12 has following legacy Unicode APIs removed and swig_int_templates.hh uses PyUnicode_GET_SIZE function that has been removed thus the error.

sharmeye commented 9 months ago

@ddj116 if you merge the latest master commit into this branch the MacOS test should pass. Go ahead and merge in the latest master commit and if it passes we'll merge this into the master branch.

ddj116 commented 9 months ago

I merged with origin/master and squashed my commits with no change to code, and now I'm getting a different set of weird failures on RHEL 8 that seem to have nothing to due with my code:

/usr/bin/g++ -g -I/usr/include -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fno-rtti -I/usr/include/udunits2 -std=c++14 -DLIBCLANG_MAJOR=16 -DLIBCLANG_MINOR=0 -DLIBCLANG_PATCHLEVEL=6 -DTRICK_GCC_VERSION=\"8.5.0\" -c PrintFileContents10.cpp -o object_Linux_8.5_x86_64/PrintFileContents10.o
In file included from /usr/include/clang/Basic/LLVM.h:21,
                 from /usr/include/clang/AST/APValue.h:16,
                 from /usr/include/clang/AST/Decl.h:16,
                 from ConstructValues.hh:9,
                 from EnumValues.hh:9,
                 from EnumValues.cpp:4:
/usr/include/llvm/Support/Casting.h:266:32: error: 'optional' is not a member of 'std'

Comparing the same build line to the last successful set of jobs, there's a difference in the value of -DLIBCLANG_PATCHLEVEL. Looks like it's 7 for the success on origin/master but 6 for this PR. Any ideas on what controls this value or if this is the root issue?

hchen99 commented 9 months ago

I merged with origin/master and squashed my commits with no change to code, and now I'm getting a different set of weird failures on RHEL 8 that seem to have nothing to due with my code:

/usr/bin/g++ -g -I/usr/include -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fno-rtti -I/usr/include/udunits2 -std=c++14 -DLIBCLANG_MAJOR=16 -DLIBCLANG_MINOR=0 -DLIBCLANG_PATCHLEVEL=6 -DTRICK_GCC_VERSION=\"8.5.0\" -c PrintFileContents10.cpp -o object_Linux_8.5_x86_64/PrintFileContents10.o
In file included from /usr/include/clang/Basic/LLVM.h:21,
                 from /usr/include/clang/AST/APValue.h:16,
                 from /usr/include/clang/AST/Decl.h:16,
                 from ConstructValues.hh:9,
                 from EnumValues.hh:9,
                 from EnumValues.cpp:4:
/usr/include/llvm/Support/Casting.h:266:32: error: 'optional' is not a member of 'std'

Comparing the same build line to the last successful set of jobs, there's a difference in the value of -DLIBCLANG_PATCHLEVEL. Looks like it's 7 for the success on origin/master but 6 for this PR. Any ideas on what controls this value or if this is the root issue?

Probably more related to the the -DLIBCLANG_MAJOR. The last success build was using -DLIBCLANG_MAJOR=15 and the failed one using -DLIBCLANG_MAJOR=16. Users have had issues with recent RHEL 8 update with llvm16. Trick llvm16 support is in work. Although both ubuntu:22.04 images used (for success and fail builds) have Clang: 13.0.1, 14.0.0, 15.0.7. Deleted this line as it is not applicable here. The fix is in master ;-)

sharmeye commented 9 months ago

@ddj116 try merging master in again now. Every time you try to get this pull brought in something like this happens. Are you sure this branch isn't cursed?