-
Notifications
You must be signed in to change notification settings - Fork 38
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
Error norms #15
base: master
Are you sure you want to change the base?
Error norms #15
Conversation
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.
This looks really good, just a few things would be great:
- I'm getting a number of compiler warnings, mostly related to the need to convert some initializer lists to use double curly braces (for subobject initialization)
- It would be nice if the --compute_errors flag was added to the --help output
- It would be nice if there was some mention of it in the README file, with the note about running with sigs=0
- This doesn't compile on GPU machines. Two options: either make it work on the GPU (this may be difficult) or make it a optional feature that can be controlled via CMake (like -DENABLE_COMPUTE_ERRORS). That way we can turn it off (and not compile it) for GPU builds
…ge message. 3) added something about the error code to the README
I believe I have addressed all the issues in the review. I made some changes so that the errors work on gpu builds. Basically I added policies for ErrorNorms that are sequential when GPU builds are enabled. I built and tested these on lassen. Please let me know if there are more corrections and/or additions to make. |
@kkhemc Thanks! I'll try and take a look at this over the weekend or next week. |
Here are some additions to Kripke that allows it to compute error norms for the type "i" problems, i.e. no scattering. You would run kripke with something like
kripke.exe --sigs 0,0,0 --zones 200,400,200 --quad 8:4 --niters 1 --compute_errors
If this is useful, we could add diagnostics comparing to the angle integrated total fluxes for the "type ii" problems. The added code has the necessary benchmark data, we would just need to add the specific diagnostic calculations.