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

CUDAEnsemble / RunPlan / RunPlanVector tests #656

Closed
4 tasks done
ptheywood opened this issue Aug 19, 2021 · 3 comments · Fixed by #665
Closed
4 tasks done

CUDAEnsemble / RunPlan / RunPlanVector tests #656

ptheywood opened this issue Aug 19, 2021 · 3 comments · Fixed by #665

Comments

@ptheywood
Copy link
Member

ptheywood commented Aug 19, 2021

Testing of CUDAEnsembles and related features is sparse / missing, with minor coverage provided in the test_logging files for each interface.

Some functions such as setRandomSimulationSeed do not appear in either test suite.

  • CUDA C++ Tests
  • CUDA C++ Fixes
  • Python tests
  • Python Fixes
@ptheywood ptheywood added this to the v2.0.0-alpha.N milestone Aug 19, 2021
@ptheywood
Copy link
Member Author

ptheywood commented Aug 26, 2021

Potential issues to ensure there are test for, as highligthed by JP (snippets written by me so may not be exact match):

Set seeds on RunPlans and maybe elsewhere appear to only support little numbers that fit int uint32_t, but most RNG implementations have 48 bits of entropy or more. We should probably modify the c++ to allow this, and not need a per int template specialisation. for it.

vec = pyflamegpu.RunPlanVector()
# This is a > 32 bit value, so will need a type specialised version of setSeed, or make it take 64 bit values all the time which is likely the better choice. 
x = 4628484339127465486
vec.setRandomSimulationSeed(x)
# OverflowError: in method 'RunPlan_setRandomSimulationSeed', argument 2 of type 'unsigned int'

SetRandomSimulationSeed methods may also have unsigned vs signed issues from python, but this may already have been resolved

vec = pyflamegpu.RunPlanVector()
# Use a small number. May encounter signed / unsigned issues.
x = 46
vec.setRandomSimulationSeed(x)

Calling SetPropertyInt(100) followed by GetPropertyInt returns 1 (the default).

@ptheywood
Copy link
Member Author

re: Random seeds of more than 32 bits:

CuRAND accepts unsigned long long for seeds, i.e. uint64_t

The current host rand implementation uses std::mt19937 which only accepts uint32_t seeds, hence seeds throuhgout flame are 32 bit values. Instead, we can use std::mt19937_64 for a host rng with more than 32 bits of entropy, that accepts uint64_t as seeds.

This should address the seed type issue in python, but will require changes throughout many files where seeds are currently considered uint32_t

@ptheywood
Copy link
Member Author

ptheywood commented Aug 27, 2021

Have now confirmed that there are issues with:

  • RunPlan::getProperty when setProperty has not been called (segfault in the c++ end)
  • RunPlan::getProperty after calling setProperty returns the old value.

@ptheywood ptheywood modified the milestones: v2.0.0-alpha.N, v2.0.0-alpha.1 Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant