-
Couldn't load subscription status.
- Fork 575
Can now print some build info #2253
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
Conversation
|
This looks super useful to me. Just wondering if we need those if statements. Are there any disadvantages to having this profle building always enabled |
Which if statements? Also, the compiler could optimize a little more without profiling enabled, but I am completely speculating on that. |
|
Ah sorry, I thought there were some if statement that made this build info optional, but looking again I can see that I misread it. This looks like a very welcome addition to me. |
|
@paulromano @jtramm just would like another opinion here. Do you think this is useful and not bloat code? I don't want to drop stuff in just because I like it. (worth asking after I typedef'd every vector in openmc and have yet to make any PRs that use it lol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is a very useful addition — thanks @gridley! In fact, I almost wonder whether we should show this information by default when a user runs OpenMC (right after we print version and git SHA1). It may make it easier for us to diagnose problems from users if we can see information on their build setup.
|
OK guys, the comments you made should be incorporated. Output now looks like this: Any further feedback? |
|
No clue what's up with CI. Does that look like something that's my fault? |
|
Originally it looks like one CI job failed due to not being able to download the nuclear date, I've rerun that job and it passed. So I've now rerun the other jobs as well. |
|
Thanks both, much appreciated feature and squeezing it in |
Allows some printing of basic build options. This way, people wouldn't have to test with a libmesh input or similar to find out if their installation supports that feature. Personally I would have found this convenient when doing an install through spack. This would also likely be useful when helping users debug their inputs.
This functionality could be expanded a little bit further. One option is to add the date (as python shows when ran in REPL mode), but that makes it so that two binaries compiled with identical options/compiler are not the same. This seemed like it wouldn't be worth it as there's probably some value to having a perfectly reproducible compile.
Another aspect is that we have both a short form and long form of command line arguments, but this seems it would be so infrequently used that it almost doesn't deserve a shortcut like -b. Maybe just --build-info would be OK, but it would stick out like a sore thumb.
Example:
Thoughts?