API update to include xintensity and rawintensity

[Project-Khepri]

Hello, it would be a good idea to include the new types of intensities in the sgminer API.

[veox]

Also in documentation.

[sshimko]

I want to discuss the API change for modifying intensities before I implement it.

I believe the most straight-forward approach that provides backwards compatibility is to allow clients to prepend "X" and "R" to the intensity value sent to the API. Then, in gpu_intensity() in api.c, if the first character is "X" or "R" parse the remainder of the char * w/ atoi(), and set the appropriate intensity for the GPU. Thoughts?

[jcw-]

The two places in the API where Intensity is exposed (that I know of) is gpuintensity and devs. In order to avoid a magic string in the value and also to support devs, perhaps something along the following lines would work?

• Add an optional third parameter to gpuintensity that specifies what kind of number is being given (e.g. X, R, I[default]). Examples:
  • gpuintensity 0, 18 Backwards compatible usage
  • gpuintensity 0, 800, X
  • gpuintensity 0, 1184000, R
• Add an additional property to the device response from devs that in addition to the numerical Intensity property, also includes an IntensityKind or IntensityType that indicates what kind of intensity the value is (e.g. the X or R or I enumeration)

[sshimko]

@jcw- thanks the first bullet sounds like a solid approach. I'll take a crack at it.

The second bullet is a path I went down but it appeared to be misleading for existing clients that expect only only one type of intensity. E.g., miner.php showing 585 in the Intensity column which is actually a X intensity. I would prefer adding another property, but is there enough value in separating it out for reads to justify the confusion for users of existing clients? What do you think about returning the magic string for APIVERSION <= 3.1 and a separate property for APIVERSION > 3.1?

[jcw-]

Yeah, that sounds like it should work. In theory you could add the property regardless (a new property shouldn't break anything). The behavior of what Intensity emits is the trick though. My only concern is that emitting devs -> Intensity as say X800 is that although it should work for clients that are just taking that as a string value and displaying it, any clients that are converting that value to a number are going to break.

Taking what you've suggested slightly farther:

• Add the new type/kind property (regardless of API version, or just for newer ones, probably doesn't really matter)
• Add a new configuration option/flag (called say, PrefixIntensity for example) that determines if devs -> Intensity will emit a number (e.g. 800) or a prefixed number (e.g. X800).
• Default PrefixIntensity to true for APIVERSION <= 3.1 and false for APIVERSION > 3.1

This extra complexity may not be needed if you think it is unlikely that clients are trying to convert it to a number - but if it's a concern, something like this would allow any older clients that are having an issue with the prefixed intensity to disable it through configuration and just live with seeing a "unitless" number in the client.

[veox]

The second bullet is a path I went down but it appeared to be misleading for existing clients that expect only only one type of intensity. E.g., miner.php showing 585 in the Intensity column which is actually a X intensity.

If a machine is running with (shader-based) xintensity or rawintensity, and the client is incapable of understanding this, then the issue is with the client. It could probably display the same intensity that sgminer uses internally (0).

For changes like these, I suppose the example clients should be updated.

[veox]

The thing with intensity is, however, a bit more convoluted.

Vanilla intensity is just a fancy way of setting the total number of (OpenCL device) threads (EDIT: that is, total work-units). Raw intensity does that directly, and X (shader-based) intensity uses the number of shaders reported by the GPU to calculate the number of threads.

Having three ways to set the same things is excessive. I was thinking of doing away with vanilla intensity and xintensity (as internally stored GPU configuration parameters) altogether, since they have insufficient granularity on either end of the setting interval, and just leaving rawintensity - but actually, rename it to threads (EDIT: or work-units).

Sure, this would create confusion with gpu-threads (host-side GPU babysitting - work validation and feeding), but nothing unresolvable.

This would be a first step to improve on what @Zuikkis outlined here.

[sshimko]

Went out of town so picking this up...

@veox it seems that you support dropping shader-based intensities in favor of raw intensities. If that is a relatively short-term goal, I'd prefer not to tackle the API changes now. If that is a more long term goal I'll consider implementing them now as described above.

[veox]

What I wrote (2nd comment) was a rambling on how I see it long-term. Shader-based intensity calculations can remain as a wrapper to raw intensity. All of this is up for discussion (I'd prefer on the mailing list, the discussion won't be gone once the issue is closed).

It is totally fine by me to implement intensities in the API now as you and @jcw- discussed.

[starlilyth]

+1. As features are added to your fork, it is critical that the API reflect the changes, and the relevant data is available for miners.

[veox]

Added https://github.com/sgminer-dev/sgminer/commit/d27e8691e1ef3c7c7147d31d04e862ad518b7fad for now, a-la bfgminer.

Should the issue be closed?