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

Error norms #15

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Error norms #15

wants to merge 6 commits into from

Conversation

kkhemc
Copy link

@kkhemc kkhemc commented May 25, 2019

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.

Copy link
Collaborator

@ajkunen ajkunen left a 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:

  1. 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)
  2. It would be nice if the --compute_errors flag was added to the --help output
  3. It would be nice if there was some mention of it in the README file, with the note about running with sigs=0
  4. 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

@kkhemc
Copy link
Author

kkhemc commented Jun 28, 2019

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.

@ajkunen
Copy link
Collaborator

ajkunen commented Jun 28, 2019

@kkhemc Thanks! I'll try and take a look at this over the weekend or next week.

@brunner6
Copy link

brunner6 commented May 5, 2024

@ajkunen and @rchen20 : This MR is really old. If this is still worth merging, we should. Otherwise we should close this. @kkhemc : I'm sorry this has taken more than a few years to evaluate...

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

Successfully merging this pull request may close these issues.

3 participants