PDENLPModels.jl icon indicating copy to clipboard operation
PDENLPModels.jl copied to clipboard

Duplicate entries in jac_structure

Open MaxenceGollier opened this issue 1 year ago • 4 comments

I have an issue when using PDENLPModels.jl.

When I call

(rows,cols) = jac_structure(nlp)

on some PDENLPModel, the tuple (rows,cols) can have duplicate entries which creates bugs on some linear algebra packages (like QRMumps.jl). I think it would be easier to add a condition in the jac_structure function to prevent this.

I have a minimal working example if needed which is just the test/problems/controleslaticmembrane.jl example with n=20.

MaxenceGollier avatar May 01 '24 15:05 MaxenceGollier

Hi @MaxenceGollier ! Sorry for the late reply.

What do you mean by "it would be easier to add a condition in the jac_structure function to prevent this"?

It is not trivial to do avoid duplicate. When we collect all the contributions from the mesh to the Jacobian, we would have to iterate over all collected values to see if it is already there. True, it saves memory, but it costs time too.

We could add a post-treatment function that given the tuple (rows, cols, vals) return another tuple without duplicate, though.

tmigot avatar May 15 '24 11:05 tmigot

Ok, I was expecting it to be too costful. I don't think adding a post-treatment function here will be useful for me. Maybe it would be useful to add some kind of warning in the documentation though ?

MaxenceGollier avatar Jun 01 '24 09:06 MaxenceGollier

Ok, why not. Any suggestion about where we could put it? I don't think the NLPModel API documentation suggest that there are no duplicate entries, but you are right that it is worth mentioning it.

tmigot avatar Jun 11 '24 21:06 tmigot

Do you have any ideas where we could add this information ? Ideally, I would add some kind of "Warning: jac_structure(nlp) can have duplicate entries which may cause errors if directly sent to LinearAlgebra libraries." I think adding a "@warn" is perhaps a bit too much, I don't know what do you think ?

MaxenceGollier avatar Jun 12 '24 10:06 MaxenceGollier