SasView
Replies: 7 comments 2 replies
-
|
Layers of configuration certainly always offer complexity. 1b (env overrides conf but is not saved) is the normal precedence for unix-like utilities; following that precedent is probably the principle of least surprise. The principle is that the value should come from as close to the user as possible: there's config file for the entire installation, environment for the particular login session, and finally there might be commandline options for that particular invocation, in order of increasing specificity and therefore priority. |
Beta Was this translation helpful? Give feedback.
-
|
If you take conf to be an extension of the GUI dialog that modifies the value for the session, it is pretty clear that it should take precedence over env so that value carries over to the next session (option 2). However, the current behaviour is "good enough", at least until another application comes along which modifies the stored SAS_OPENCL environment variable to control sasmodels. |
Beta Was this translation helpful? Give feedback.
-
|
I think I reproduced the original behaviour with my recent pr - definitely
something that I would like to revisit soon.
…On Fri, 30 Sept 2022, 00:54 Stuart Prescott, ***@***.***> wrote:
Layers of configuration certainly always offer complexity.
1b (env overrides conf but is not saved) is the normal precedence for
unix-like utilities; following that precedent is probably the principle of
least surprise. The principle is that the value should come from as close
to the user as possible: there's config file for the entire installation,
environment for the particular login session, and finally there might be
commandline options for that particular invocation, in order of increasing
specificity and therefore priority.
—
Reply to this email directly, view it on GitHub
<#2226 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACKU4SXSO6J2ZE2AKVSSITDWAYT2XANCNFSM6AAAAAAQY6ENMA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
|
Is this a freaky coincidence or has something gone awry in what someone has done in the last few days, but all my installed 5.x releases just stopped starting; I got the dreaded 'unable to execute script sasview'. EXCEPT 5.0.5 popped up a nice little box and told me why: there was a syntax error in my custom_config.py: Inserting a CR between the declarations has rectified the problem. |
Beta Was this translation helpful? Give feedback.
-
|
This shouldn't happen after #2168 |
Beta Was this translation helpful? Give feedback.
-
|
Interesting! I'm going to guess that you have been testing some of the recent branches? or main? I think there may have been some changes to the config.py? And since that is in the user space, all versions use the same custom_config.py. My best guess. Maybe something to keep in mind when reviewing things that alter files in the user .sasview directory? |
Beta Was this translation helpful? Give feedback.
-
That would be my vote. Environment variables are not a great place for the data, which can be passed between sasview and sasmodels in other ways. |
Beta Was this translation helpful? Give feedback.
-
|
Not for long though!
…On Fri, Sep 30, 2022 at 2:29 PM Paul Butler ***@***.***> wrote:
Interesting! I'm going to guess that you have been testing some of the
recent branches? or main? I think there may have been some changes to the
config.py? And since that is in the user space, all versions use the same
custom_config.py. My best guess. Maybe something to keep in mind when
reviewing things that alter files in the user .sasview directory?
—
Reply to this email directly, view it on GitHub
<#2226 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACKU4STLGJJNMVS46V7XFXDWA3TLXANCNFSM6AAAAAAQY6ENMA>
.
You are receiving this because you commented.Message ID:
***@***.***>
--
Dr Lucas Wilkins
+44 (0) 7505 915 726
Personal Website: http://www.lucaswilkins.com/
Alternate e-mail: ***@***.***
|
Beta Was this translation helpful? Give feedback.
-
There are complex interactions between the SAS_OPENCL environment variable and the SAS_OPENCL key stored user configuration. Call these env and conf respectively so we don't get confused. This discussion assumes the new configuration management code which is not yet merged.
env exists because it allows the sasmodels library to be controlled from outside. For example, when using remote batch fitting on a GPU cluster where there are multiple devices per cluster you need to tell sasmodels which device was allocated to the fit by the queuing program. Rather than building it into the packages that use sasmodels it is easier to control through the environment. For example, an HPC system with cuda but no opencl can define env as SAS_OPENCL="cuda:$CUDA_VISIBLE_DEVICES" where CUDA_VISIBLE_DEVICES is set by the slurm queuing system.
SasView currently has env taking precedence over conf on startup. Because the GPUOptions dialog does not provide an option for cuda drivers this is the only way to test sasview with cuda. The external env will change the stored conf if it does not match the default value of "none". This makes the environment variable "sticky". If you rerun without the environment variable set and without setting GPUOptions then it will use the old env rather than the old conf. Even if a new device is selected in the GPUOptions it will only override env for the remainder of the session. The new value will be saved to conf but the env will take precedence the next time SasView is started.
We have some options: (1) have env take precedence over conf (a) with or (b) without saving env to conf, (2) have env provide the default for conf with stored conf taking precedence, or (3) ignore env and only use conf. The current implementation using (1a) is probably good enough. Those users who bother to set the environment variable can handle the complexity. There is currently no need to select cuda from GPUOptions. For sasmodels code OpenCL and CUDA have approximately equal performance and the OpenCL drivers are installed along with the CUDA drivers on Windows. There is a chance that a separate windows application using sasmodels decides to set set SAS_OPENCL environment during windows install which will cause unexpected "forgetfulness" in SasView. Logging the source of the GPU device selection (env, stored conf or default) will help when debugging. If necessary we could set a permanent value for env for Windows from the GPUOptions dialog.
It may make the code easier if we default conf to env on startup and it will help reduce stickiness. It will reduce confusion if we rename conf to GPU_DEVICE so that it is clear that this variable has a different lifecycle from env. Whether we honour conf values from old config files or tell users that they need to re-select the GPU device is a separate question (the re-select option requires less code). Another source of confusion is that sasview controls sasmodels through env. We could instead add a function to
sasmodels.coreto change the gpu device or use conf to specify the device when loading the model.Future maintainers will be looking at the config variables to debug issues. We can provide developer docs in the code to help them along. Something like:
I don't think this level of detail is needed for the user documentation.
Consider moving code from GPUOptions to sasmodels. Have sasmodels return the list of available devices and provide a test iterator to produce the results. The Qt GUI will select the device, trigger the tests, display the results, and update conf.
We still do not have a good solution for sasmodels on Apple M1.
Beta Was this translation helpful? Give feedback.
All reactions