Update to AMGX_jll 2.4 and later Julia versions #33
Conversation
src/Config.jl
Outdated
| if json | ||
| str = sprint(JSON.print, d) | ||
| else | ||
| str = "" |
There was a problem hiding this comment.
Better to use an IOBuffer or a sprint() do call here.
src/Config.jl
Outdated
| @@ -18,8 +18,15 @@ function create!(config::Config, content::String) | |||
| end | |||
| Config(content::String) = create!(Config(), content) | |||
|
|
|||
| function create!(config::Config, d::Dict) | |||
| str = sprint(JSON.print, d) | |||
| function create!(config::Config, d::Dict; json = false) | |||
There was a problem hiding this comment.
The create! functions are not documented in the README so they are purely internal it seems? But then this kwarg doesn't do anything since the default will always be used?
There was a problem hiding this comment.
It also changes the default from JSON to non JSON. Why is that desirable?
There was a problem hiding this comment.
The change was added by @vchuravy in the branch, but when I tested it seemed like the right choice since the JSON parsing seemed a bit brittle. AMGX will always try parsing as JSON first, and then the string format. I added the (not exposed) kwarg to have an option to fall back to the old way just in case if someone wants to make use of internals. Not sure how to best handle this, but the tests do not pass with the old JSON formatter.
There was a problem hiding this comment.
I have removed the json option in the latest version. I wonder if the JLL might be built without JSON parsing working - I cannot get it to load the test JSON files from the AMGX repository. There is a sort of related issue in the AMGX repository NVIDIA/AMGX#99 where one of the first checks was to see if RAPIDJSON_DEFINED is in the build flags. CMake should have found it when building since it is bundled with the AMGX repo. I am able to set most options using the fallback string interface now at least, so the library is functional.
|
I have some comments but they can be done in a follow up PR. |
| @@ -42,6 +42,12 @@ function setup!(solver::Solver, matrix::AMGXMatrix) | |||
| return solver | |||
| end | |||
|
|
|||
| function resetup!(solver::Solver, matrix::AMGXMatrix) | |||
There was a problem hiding this comment.
How are people supposed to know about this function and know what it does? It isn't in the README I think?
This is a continuation of PR #28. The tests pass on Julia 1.11. I modified some of the tests, as it appear that unless
monitor_residualis explicitly set, the status codes are not updated. There is also a new (?)NOT_CONVERGEDthat the test that previously diverged returns.I also added the option to either pass JSON or the string when making the config since there are two versions of this input in main AMGX now.
I did not touch the buildkite setup since I am not 100% sure on what combinations of Julia and CUDA are natural to test.