[boost-ublas] Add opencl feature

[sweemer]

• Changes comply with the maintainer guide
• SHA512s are updated for each updated download
• The "supports" clause reflects platforms that may be fixed by this new version
• Any fixed CI baseline entries are removed from that file.
• Any patches that are no longer applied are deleted from the port's directory.
• The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
• Only one version is added to each modified port's versions file.

boost-ublas has a dependency on clBLAS but this dependency is not reflected in vcpkg.json. Furthermore, the existing dependency on boost-compute is only required when clBLAS is used. To make this dependency clear, I am adding the opencl feature, which is off by default to preserve the current behavior.

[BillyONeal]

1. Changes that affect boost need to touch scripts/boost/generate-ports.ps1; the changes here will be overwritten next time that script is called.
2. The feature needs to actually be hooked up to something. That is:

   vcpkg install boost-ublas[core]
   vcpkg install boost-compute
   

   and

   vcpkg install boost-compute
   vcpkg install boost-ublas[core]
   

   need to give you the same results but it looks like you're saying the latter would cause boost-ublas to do something else.

[BillyONeal]

Probably makes sense to add right below boost-regex here:

vcpkg/scripts/boost/generate-ports.ps1

Line 136 in 74e940f

"boost-regex" = @{

[sweemer]

I've made the suggested change to scripts/boost/generate-ports.ps1 and when I ran the script it made some changes to some other boost packages, such as removing the port-version field. Is that expected?

Also, I don't quite follow the point about getting different results depending on the order of installing boost-compute and boost-ublas[core]. As far as I understand both of those sequences would result in the same outcome. Could you elaborate a bit on why you think the result might be different?

[BillyONeal]

I've made the suggested change to scripts/boost/generate-ports.ps1 and when I ran the script it made some changes to some other boost packages, such as removing the port-version field. Is that expected?

Should be fixed by #29338 :)

Also, I don't quite follow the point about getting different results depending on the order of installing boost-compute and boost-ublas[core]. As far as I understand both of those sequences would result in the same outcome. Could you elaborate a bit on why you think the result might be different?

There's no code here to hook the feature selection into boost's build system, so it looks like you're relying on some form of autodetection going on inside b2. That is, it looks like:

vcpkg install boost-ublas[core]
vcpkg install boost-compute

Gives you a ublas without the feature, but

vcpkg install boost-compute
vcpkg install boost-ublas[core]

would effectively give you boost-ublas[opencl] even though the user didn't ask for the opencl feature.

[BillyONeal]

Cancelled the PR build because the fleet is very behind today and there are merge conflicts with this PR outstanding.

[sweemer]

I've rebased my branch on master (not yet pushed) and now the problem with the port-version field being removed is resolved, but I now have the opposite problem, which is that generate-ports.ps1 updates all the vcpkg.json file for all boost packages that depend on boost-ublas with the new version (i.e. 1.81.0#2) without also updating the port-version for those packages. I believe it's due to this logic in generate-ports.ps1:

            if ($dependency.StartsWith("boost")) {
                $dependency = @{
                    "name"       = $dependency
                    "version>="  = MakePortVersionString $dependency
                }
            }

It seems to me that generate-ports.ps1 should either update both the port-version and the dependency version, or update neither. If it helps illustrate the point I can push my branch in its current (broken) state, just let me know.

[BillyONeal]

now have the opposite problem, which is that generate-ports.ps1 updates all the vcpkg.json file for all boost packages that depend on boost-ublas with the new version (i.e. 1.81.0#2) without also updating the port-version for those packages

Unfortunately you just have to list all of them :(

I can help with the generate-ports part but I'm not helpful with the feature binding part; if you could look at that it'd be a more effective way to move this submission forward.

[FrankXie05]

Pinging @sweemer for response. Is work still being done for this PR?

[sweemer]

HI @FrankXie05 yes I am still planning to work on this, but it is taking longer than I expected to fully understand how the boost build configurations work.

If you want then I can close this pull request and open it back up later once I've figured out the solution.

[FrankXie05]

@sweemer You can convert to draft. :)

[sweemer]

I've rebased this branch onto master and have removed the draft status.

There's no code here to hook the feature selection into boost's build system, so it looks like you're relying on some form of autodetection going on inside b2. That is, it looks like:

vcpkg install boost-ublas[core]
vcpkg install boost-compute

Gives you a ublas without the feature, but

vcpkg install boost-compute
vcpkg install boost-ublas[core]

would effectively give you boost-ublas[opencl] even though the user didn't ask for the opencl feature.

I still don't fully understand what you're trying to say here. The "opencl feature" is nothing more than just pulling in some more dependencies, thirteen to be precise:

$ ./vcpkg depend-info boost-ublas[core] | wc -l
67
$ ./vcpkg depend-info boost-ublas[opencl] | wc -l
80

As far as I am concerned, there's nothing more to it than that. If the user also installs boost-compute for some reason, then yes, they will end up with the additional dependencies, but that's not really related to my change. The whole point of this change is to avoid bringing in the additional dependencies by default, as I mentioned in the original description. Or am I still missing the point?

[BillyONeal]

I still don't fully understand what you're trying to say here. The "opencl feature" is nothing more than just pulling in some more dependencies

That is, in fact, the problem. Whether the feature is on needs to be controlled by whether the feature is on, not based on which dependencies happen to be installed first.

If those 13 dependencies happen to be installed first and the feature is off, the feature needs to be off. Otherwise:

vcpkg install the 13 dependencies
vcpkg install boost-ublas[core]

and

vcpkg install boost-ublas[core]
vcpkg install the 13 dependencies

would give different answers because we aren't rerunning the boost-ublas build because dependencies it did not declare changed.

[dg0yt]

Mind the binary caching. The cache key can only consider explicit dependencies. Any dependencies which are picked by accident at build time make the binary artifact unusable because the dependencies won't be installed when the artifact is reused.

[sweemer]

If those 13 dependencies happen to be installed first and the feature is off, the feature needs to be off.

Why isn't that the case now? I'm usually not this dull, I promise, but it seems that I'm missing something basic despite my best efforts to understand how this works.

Any dependencies which are picked by accident at build time

I wasn't aware that this was possible. How might it happen?

At this point I feel like it might be worthwhile for me to take a step back and restate what my original goal is: I don't want boost-ublas to depend on boost-compute by default, but it should be added as a dependency when the OpenCL features are optionally enabled. Am I going about accomplishing this goal in the right way?

[BillyONeal]

I wasn't aware that this was possible. How might it happen?

Everything that is installed before a port is available for consumption by that port, which means any 'optional try to detect what's available' in ports is problematic. If there's a feature to control the behavior, it needs to explicitly control the behavior, not make stuff available and expect the build system to do something reasonable with that.

How does boost-ublas control this behavior before this PR?

At this point I feel like it might be worthwhile for me to take a step back and restate what my original goal is: I don't want boost-ublas to depend on boost-compute by default, but it should be added as a dependency when the OpenCL features are optionally enabled. Am I going about accomplishing this goal in the right way?

You need both this change and something that actually connects the feature. That is, if the feature is off, boost-ublas needs to behave as if boost-compute is not installed, even if it actually is installed.

[sweemer]

How does boost-ublas control this behavior before this PR?

As far as I can tell, boost-ublas doesn't make any attempt to control the OpenCL behavior before this PR - it just expects the headers to be available, even though they aren't by default. As I mentioned in the description at the top, the dependency on clblas is missing, so if you try to #include <boost/numeric/ublas/opencl.hpp>, you will get the following error:

[build] E:\project\build\windows-msvc\vcpkg_installed\x64-windows\include\boost\numeric\ublas\opencl\library.hpp(13,10): fatal  error C1083: Cannot open include file: 'clBLAS.h': No such file or directory 

So to recap, here is the situation for users of the OpenCL functionality before my PR:

# doesn't work
vcpkg install boost-ublas

# works
vcpkg install clblas
vcpkg install boost-ublas

# also works
vcpkg install boost-ublas
vcpkg install clblas

You need both this change and something that actually connects the feature. That is, if the feature is off, boost-ublas needs to behave as if boost-compute is not installed, even if it actually is installed.

The only difference in the behavior of boost-ublas with the clblas and boost-compute dependencies installed and without them is that users can successfully include headers from the boost/numeric/ublas/opencl directory. If users don't try to include any headers from this directory then there is no difference at all.

Are you suggesting that when the opencl feature is off, the boost/numeric/ublas/opencl header directory should be excluded from the install? Apart from that, I can't think of any other way to force boost-ublas to behave as if clblas and boost-compute are not installed.

[FrankXie05]

Temporarily converted to draft, waiting for BillyONeal re-review.

[sweemer]

Hi @FrankXie05 why does this PR have to be converted to a draft? I'm still working on it actively and hope it can be merged soon.

[FrankXie05]

@sweemer
Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review".
That way, I can be aware that you've responded since you can't modify the tags.

[sweemer]

I have converted back to ready for review, and will await a response from @BillyONeal on the questions above.

[sweemer]

Hi @BillyONeal can you help me move this PR forward by responding to my latest post #29325 (comment)?

[BillyONeal]

In that case I'm not seeing why this is a feature; it seems like opencl users should be adding the opencl dependency themselves?

[sweemer]

It may not be clear to users which dependencies are required to use opencl with boost-ublas, so I can see value in providing the feature for users.

Having said that, removing the feature makes things easier for me, as it reduces my PR to simply removing boost-compute as a dependency, because as mentioned in my original description, this dependency is already broken.

Please let me know if you still prefer to remove the feature and I will make the change. Or if you'd rather keep it, then please help merge my changes.

[BillyONeal]

Please let me know if you still prefer to remove the feature and I will make the change. Or if you'd rather keep it, then please help merge my changes.

If the feature doesn't control anything about this port then I don't think it should be a feature of this port.

[sweemer]

Ok, fair enough. I have simply removed boost-compute as a dependency for now. Users will need to add it as a dependency of their project along with clBLAS if they want to use the OpenCL functionality from boost-ublas.

[dg0yt]

AFAIU the current behaviour of the port is already broken. Users can't control the installation order (unless using an overlay i.e. fixed port). The feature would only fix the case when it is actually use. Without a configuration option in the package, or a patch, the only consistent implementation would be permanent dependencies on the packages which are otherwise picked up randomly. I don't know if this desired.

To make it clear: there is no blame on your contribution. The problem is in the upstream package and/or our incomplete understanding how to actively control its dependencies.

[sweemer]

I have rebased my branch on the latest master. @BillyONeal Please help merge as soon as you get a chance.

[BillyONeal]

Thanks, and sorry for the confusion and delay on my part :)