Opened 9 years ago
Last modified 6 years ago
#564 reopened defect
improve support for tgamma and other missing function
Reported by: | smk78 | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | SasView Next Release +1 |
Component: | SasView | Keywords: | |
Cc: | Work Package: | SasModels Infrastructure |
Description
Executing make clean html from Windows CMD or Git Bash shell:
python genmodel.py ../sasmodels/models/fractal.py model/fractal.rst
Traceback (most recent call last):
File "genmodel.py", line 103, in <module>
plot_1d(model, opts, ax1d)
File "genmodel.py", line 47, in plot_1d
Iq1D = calculator()
File "e:\sasviewdevelopment\sasmodels\sasmodels\direct_model.py", line 214, in call
return self._calc_theory(pars, cutoff=self.cutoff)
File "e:\sasviewdevelopment\sasmodels\sasmodels\direct_model.py", line 174, in _calc_theory
self._kernel = self._model.make_kernel(self._kernel_inputs) # pylint: disable=attribute-dedata_type
File "e:\sasviewdevelopment\sasmodels\sasmodels\kernelcl.py", line 320, in make_kernel
self.fast)
File "e:\sasviewdevelopment\sasmodels\sasmodels\kernelcl.py", line 224, in compile_program
program = compile_model(self.get_context(dtype), source, dtype, fast)
File "e:\sasviewdevelopment\sasmodels\sasmodels\kernelcl.py", line 150, in compile_model
program = cl.Program(context, source).build(options=options)
File "c:\Anaconda\lib\site-packages\pyopencl\init.py", line 213, in build
options=options, source=self._source)
File "c:\Anaconda\lib\site-packages\pyopencl\init.py", line 253, in _build_and_catch_errors
raise err
pyopencl.RuntimeError?: clBuildProgram failed: build program failure -
Build on <pyopencl.Device 'Oland ' on 'AMD Accelerated Parallel Processing' at 0xa9f5210>:
"C:/Users/smk78/AppData/Local/Temp\OCL7C94.tmp.cl", line 210: error:
storage-class specifiers extern/static/register not allowed for
variables
static float SQTPI = 2.50662827463100050242E0f;
"C:/Users/smk78/AppData/Local/Temp\OCL7C94.tmp.cl", line 232: error:
identifier "SQTPI" is undefined
y = SQTPI * y * w;
"C:/Users/smk78/AppData/Local/Temp\OCL7C94.tmp.cl", line 238: error:
storage-class specifiers extern/static/register not allowed for
variables
static float LOGPI = 1.14472988584940017414f;
"C:/Users/smk78/AppData/Local/Temp\OCL7C94.tmp.cl", line 291: warning: goto
statement may cause irreducible control flow
goto small;
"C:/Users/smk78/AppData/Local/Temp\OCL7C94.tmp.cl", line 299: warning: goto
statement may cause irreducible control flow
goto small;
3 errors detected in the compilation of "C:/Users/smk78/AppData/Local/Temp\OCL7C94.tmp.cl".
Frontend phase failed compilation.
(options: -I c:\anaconda\lib\site-packages\pyopencl\cl)
(source saved as c:\users\smk78\appdata\local\temp\tmpbnhiza.cl)
make: * [model/fractal.rst] Error 1
smk78@NDW1000 /e/sasviewdevelopment/sasmodels/doc (master)
$
Change History (6)
comment:1 Changed 9 years ago by smk78
comment:2 Changed 9 years ago by smk78
- Resolution set to fixed
- Status changed from new to closed
comment:3 Changed 9 years ago by pkienzle
- Resolution fixed deleted
- Status changed from closed to reopened
Although the docs are building there is still a warning from the compiler that code including goto is being run on the GPU.
The problem stems from a patch to extend MSVC with the tgamma function which is showing up in OpenCL even though it is not needed there. The code includes some goto statements which are not supported by GPUs.
The test for whether the code is included uses #ifdef tgamma. At a guess, some platforms define tgamma as a #define to some internal implementation, but apparently not all opencl headers (Radeon Nano on Linux for example). We can instead put a #define NEED_TGAMMA in the compilers that needed it (tinycc and msvc on windows), then test that flag to see if we really need to provide an implementation of tgamma. Similarly for expm1. This will also make the headers a little cleaner.
The sas_gamma = tgamma(x+1)/x definition is still needed, even with the replacement tgamma. IIRC, this form is more numerically stable for x<1. Ideally, I would like this to become the definition of tgamma, and not use the sas_gamma name in the code. Maybe the following in the header?
#if defined(NEED_TGAMMA) … cephes tgamma … #endif // NEED_TGAMMA // sas_gamma uses tgamma from the environment, or the cephes version defined above static double sas_gamma(double x) { return tgamma(x+1.)/x; } // protect against the tgamma definition being a macro expansion #if defined(tgamma) #undef tgamma #endif #define tgamma(_v) sas_gamma(_v)
That way those who are writing models can just use tgamma but still have it be accurate.
Note that tgamma might be defined as a macro in order to support type generic float tgamma(float) etc. This isn’t actually an issue for us because I’m substituting the word double for float for single precision, so the tgamma picked up from the environment will be the tgamma appropriate for the type.
I prefer to have tgamma/expm1 in the kernel header section rather than the library because it makes it easier to steal the header code and use it in other projects, but I’m okay with treating it more like a library as well, along with sinc, square and cube.
Some compilers are complaining about the unused static defs for sinc, square and cube. We could play at being a linker as well, and only include them if the source code uses those names. Similarly for tgamma and expm1 if we need to include our own versions.
comment:4 Changed 9 years ago by pkienzle
- Milestone changed from SasView 4.0.0 to SasView Next Release +1
- Priority changed from blocker to minor
- Summary changed from sasmodels doc build is failing on W7 to improve support for tgamma and other missing function
The blocker aspect has been removed: opencl again uses the built-in tgamma function.
Still need to provide single precision versions for completeness in case users want single precision in DLLs.
Still would be nice to make a fake linker so that only the needed functions are included.
Still would be nice to make tgamma accurate rather than the user knowing they need to call sas_gamma instead.
comment:5 Changed 6 years ago by smk78
- Work Package changed from SasView Documentation to SasModels Redesign
This is no longer a documentation task. Reassigning it to SasModels Redesign.
comment:6 Changed 6 years ago by butler
- Work Package changed from SasModels Redesign to SasModels Infrastructure
Resolved by PK's commit e07b470c1b10dba7e48f01177a6f28fec78e3259.