roystgnr / MetaPhysicL

Metaprogramming and operator-overloaded classes for numerical simulations
Other
22 stars 12 forks source link

Output desired size when AD container size is exceeded #84

Open GiudGiud opened 2 years ago

GiudGiud commented 2 years ago

It would be convenient to tell users which size to resize to

This is coming up here https://github.com/idaholab/moose/discussions/22191#discussioncomment-3737945

GiudGiud commented 2 years ago

does metaphysicl allow verbose error messages?

roystgnr commented 2 years ago

We don't have a macro for it, but we ought to add one.

roystgnr commented 2 years ago

I'm not sure it's going to be possible to tell users which size to resize to. We hit that error and exit the first time we try to resize to anything larger than the current size, so we won't keep going to find out what the final required size is going to be. Add an ADReal with (dense, to simplify the example) gradient indices of [0,20) to a vector with indices of [15,35) and if your max sparse vector size is 30 the program is going to bail there and tell you needed 35, not wait around to find out that your code was later going to add a third vector with indices of [30,50) and a fourth with [45,65).

I almost said "can't" rather than "won't" keep going, because I'm wondering if there's anything we can do here to actually keep going until we're done.

Looking at DualReal.h, for one thing we really ought to have a DynamicSparseNumberArray option, not just SemiDynamicSparseNumberArray and NumberArray. "There is always some sparsity size which will make your program scream and die" is a design choice there for efficiency, not a MetaPhysicL-imposed requirement. And, that said... perhaps we could add a compile-time option to DSNA that updates a global maximum_sparsity_size with every resize? The additional potential performance issues (We'd need to do it atomically! Atomic max/min aren't in C++20; we'd need a loop!) make me cringe, but from a user interface perspective it would be nice to be able to say "recompile this way and run, then at the end of the program you'll see exactly how you'd need to recompile a different way to get your speedup back for this particular problem".

lindsayad commented 2 years ago

Especially given the recent timings in https://github.com/libMesh/MetaPhysicL/pull/29 I think it definitely makes sense to have DynamicSparseNumberArray be a configuration option for MOOSE. Perhaps it should even be the default depending on the application

GiudGiud commented 2 years ago

it could be part of our MOOSE makefile. When you include a module that needs a high number, we ask you to reconfigure?

lindsayad commented 2 years ago

Maybe but that is a pain for folks not using the AD portions of applications