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

Negative species should trigger a retry #1952

Closed
zingale opened this issue Jul 31, 2021 · 6 comments
Closed

Negative species should trigger a retry #1952

zingale opened this issue Jul 31, 2021 · 6 comments

Comments

@zingale
Copy link
Member

zingale commented Jul 31, 2021

Right now we abort if any of the species are really negative in clean_state. We should instead do a retry

@zingale
Copy link
Member Author

zingale commented Jul 31, 2021

this really seems to be an issue with SDC for S_new after we apply the sources

@zingale
Copy link
Member Author

zingale commented Sep 9, 2021

I think that the easiest way to do this is to have clean_state return a status, which indicates if that species check failed, and if it does, we modify the advance_status status variable accordingly in Castro_advance_ctu.cpp.

Thoughts?

@maxpkatz
Copy link
Member

I prefer not to handle it exactly this way -- I would rather have it be that any update which updates the species checks at the time of the update whether it is creating an invalid state, and then reject the step at that point. Everything in clean_state() should be a backstop which resets the fluid state to something reasonable if all else fails, but we should be more rigorous about validating the output of the various updates.

Also, seems like an issue that we are only aborting in the CPU build for this error.

@zingale
Copy link
Member Author

zingale commented Sep 10, 2021

yes, we should abort on the GPU as well. Does abort work there?

@maxpkatz
Copy link
Member

It's not ideal to handle it that way -- the way to do it would be to reduce over the minimum and maximum X found, and apply the check after the parallel for.

@maxpkatz
Copy link
Member

(We would still lose the print, although we could probably rewrite it to be a GPU-safe print since either way this kernel will not be that expensive.)

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

No branches or pull requests

2 participants