Closed GediminasMasaitis closed 5 years ago
About all the the changes made in project MHArmory.Search
, as we discussed on Discord, I want to reorganize all this in order to allow search engine extensions without changing the core layers.
So I will still review your changes, but we will need to work closely to re-integrate your modifications in my code base after I will re-structure it.
In file MHArmory.Search/OpenCL/search.cl
please replace all tabs by spaces.
Also please make sure all the if
, for
and so on respect the spacing rules, such as if<SPACE>(
and for<SPACE>(
and so on.
Integration in file MHArmory.Search/Solver.cs
is going to be very different, as I said before. We will work closely to integrate it correctly after I rework the search engine usage.
What's the chances of this PR getting merged? I tried the OpenCL solver and got a ridiculous speed up (0.13 s search time) versus the current solver ( 65 s search time) for the same exact skill list. It basically makes the searches instantaneous (and even faster than Athena's Armor Set Search).
I want to merge all the wonderful work of @GediminasMasaitis but we are still discussing the details. Depending on him, this is going to be merged when finalized.
By the way for your information, the OpenCL solver might be replaced by a C# implementation that is almost as fast. In Armory the solvers are extensions (built-in for code safety) but it is relatively easy to add another implementation, so if you know about programming and algorithm, feel free to provide something great :)
@TanukiSharp Sounds good. Is there any documentation on adding a solver?
There isn't yet, but I will write one, shouldn't too hard.
Hi @DeveloperPaul123
Here is a first version of the documentation about a solver, and how to write one. I'm quite happy with what I wrote after several self reviews, but feel free to reach out is something is not clear to answer your question and refine even further the document.
https://github.com/TanukiSharp/MHArmory/blob/master/MHArmory.Search/Documentation/README.md
Maybe @GediminasMasaitis can also provide a feedback if not too busy ;)
Hello @TanukiSharp
Thanks for this! I'll be sure to take a look and get back to you with any feedback I have.
Thanks.
Feel free to open an issue to log all what's not clear (if any), so we can discuss those points :)
@GediminasMasaitis I'm closing this PR as you discussed on Discord. This is kind of redundant because your other solver is blazing fast, more reliably in term less memory-restrained, and the fact that OpenCL C# binding project is kind of dead, to name a few.
We can still re-open if something about OpenCL changes in the future.
Implementation on separate fork