Drop Coordinate Basis from IJacobian (without breaking IDL)

[KennethEJansen]

On some platforms, there is a large performance drop for having coordinate basis in IJacobian. This branch will compute IDL coefficient in IFunction that has access to coordinates, append this to jac_data so that IJacobian can avoid pulling coordinates through basis.

[KennethEJansen]

This is 2 files and only about 10ish lines of code. The place I could use some help is that currently I have bumped jac_data by 1 whether we have IDL or not. I assume we don't want that performance penalty but it is currently a declared constant so I guess we would need to make it not that so that as soon as we can use context->idl_enable to decide to bump it up?

[KennethEJansen]

It looks like I finally got all the spaces in the right places for the style requirements.

This is following exactly @jedbrown suggested mod. I don't yet see a way to only increase the size of jac_data when IDL is needed so it is just on all the time.

[KennethEJansen]

I think I figured out how to do the bump by 1 on jac_data only when idl_enable is true.

[KennethEJansen]

FWIW, we are setting up restriction operators for jac_data without checking that it is implicit. I am sure its not a big deal but just an observation.

[KennethEJansen]

Passing all tests before the last style spacing change so I think this is now complete (assuming there is not a better way than pulling the context out to determine if idl_enabled). I suppose we could also wrap the jac_data restriction creation in the same conditional to save a trivial amount of setup work for explicit problems (had to do it here as the context I am checking does not exist for explicit problems).

[jrwrigh]

A few minor style comments. Otherwise good to me!

[KennethEJansen]

So the suggested replacement (that I took without checking) with StoredValuesPack does not compile. I am going to revert that change. Here is the error

In file included from /lus/gila/projects/PHASTA_aesp_CNDA/MainInOrder/libCEED/examples/fluids/problems/newtonian.c:11:
/lus/gila/projects/PHASTA_aesp_CNDA/MainInOrder/libCEED/examples/fluids/problems/../qfunctions/newtonian.h:234:38: error: passing 'const CeedScalar' (aka 'const double') to parameter of incompatible type 'const CeedScalar *' (aka 'const double *'); take the address with &
       StoredValuesPack(Q, i, 14, 1, sigma, jac_data);
                                     ^~~~~
                                     &
/lus/gila/projects/PHASTA_aesp_CNDA/MainInOrder/libCEED/examples/fluids/problems/../qfunctions/utils.h:195:117: note: passing argument to parameter 'values_at_qpnt' here
CEED_QFUNCTION_HELPER int StoredValuesPack(CeedInt Q, CeedInt i, CeedInt start, CeedInt num_comp, const CeedScalar *values_at_qpnt,
                                                                                                                    ^
1 error generated.

[jrwrigh]

Yep, you're right. The line needs to be:

StoredValuesPack(Q, i, 14, 1, &sigma, jac_data);

[jrwrigh]

Also, for documentations sake, what were the performance gains we saw moving to this? IIRC It was around 4-8% on Polaris and SunSpot also saw a significant improvement (I don't think "percent improvement" is under NDA, but being on the cautious side of things).