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

Improve CUDAEnsemble's error reporting. #839

Merged
merged 1 commit into from
May 18, 2022
Merged

Conversation

Robadob
Copy link
Member

@Robadob Robadob commented Apr 27, 2022

Have added 3 tests to TestCUDAEnsemble suite, one for each error level, to confirm it works.

Docs repo PR needs merging at same time: FLAMEGPU/FLAMEGPU2-docs#85

@Robadob Robadob self-assigned this Apr 27, 2022
@Robadob Robadob marked this pull request as draft April 27, 2022 13:11
@Robadob Robadob force-pushed the cuda_ensemble_errors branch from c7b693e to 69972c0 Compare April 27, 2022 13:19
@Robadob Robadob force-pushed the cuda_ensemble_errors branch 2 times, most recently from 26a776d to d13673f Compare May 17, 2022 12:17
@Robadob Robadob requested a review from ptheywood May 17, 2022 12:17
@Robadob Robadob marked this pull request as ready for review May 17, 2022 12:17
@Robadob Robadob force-pushed the cuda_ensemble_errors branch from d13673f to bd7b2db Compare May 17, 2022 12:21
Copy link
Member

@ptheywood ptheywood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do with test(s) for the changes to CUDAEnsemble::checkArgs, verifying all documented accepted values work (and set the correct error level) plus atleast one test of bad input (bad integer, bad string, missing value).
Short and long forms would also be good.

Otherwise, the changes look good though I've not compiled or ran the code

@Robadob
Copy link
Member Author

Robadob commented May 17, 2022

Could do with test(s) for the changes to CUDAEnsemble::checkArgs, verifying all documented accepted values work (and set the correct error level) plus atleast one test of bad input (bad integer, bad string, missing value). Short and long forms would also be good.

That's reasonable, didn't think of that. Will do.

Edit: Useful too, arg parsing was bugged.

With 4 additional tests that the 3 levels of error reporting, and related argument parsing.
@Robadob Robadob force-pushed the cuda_ensemble_errors branch from bd7b2db to 14df52f Compare May 17, 2022 17:30
@Robadob Robadob requested a review from ptheywood May 17, 2022 17:30
@Robadob Robadob merged commit a7f9cc9 into master May 18, 2022
@Robadob Robadob deleted the cuda_ensemble_errors branch May 18, 2022 15:03
@Robadob Robadob mentioned this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants