improve generated code for import

This improves the generated code for loading vector export and scalar export.

Expected but missing fields (previously didn't exist or were @exportoffd) no longer error immediately. It is up to the constructor of the e.g. Particle type (or importconstruct) to implement a sensible default.

In addition to that this change improves load time with 20k particles by x5-20 locally for me.

This should make some of the fix_import functions obsolete.

This breaks the matching done in add_scalar_fallback. This could now be done in importconstruct or alternatively adapt the matching. add_vector_fallback will also no longer match. An importconstruct would be easiest e.g. InPartS.ExportUtils.importconstruct(t::Type{<:MotileCell}; ∫Fax=NaN, kwargs...) = t(; ∫Fax, kwargs...)

Compat with all cell models and fix_import will need to be ensured.

requested review from @pbittih

changed the description

Nice! I guess the speedup is simply from having the loop calling the constructor behind a function barrier?

I am wondering if it's somehow possible to reduce the complexity of the generated expressions (all the branching, etc), so it's easier to understand if we ever come back to this, and also easier to parse if we still need to.

There was some function barrier before already. But there was some place where the compiler still gave up, dispatched dynamically and had an assert afterwards....

It is possible. I added some of the complexity to make the generated code more easy to read...

Yeah, I like that in principle. Perhaps the import fixes that remain to be necessary won't have a problem with it.

I think we will open a MR on CGM where we co-develop the import fixes and test this InPartS MR, then we can probably converge on something that's manageable.

How do we deal with the warnings/safeguards? Besides particles and params, we also have more and more other types that are InPartS-exportable, and some of them might have simple numerical parameters with default values. If those get used accidentally, because some field is missing/renamed, it might slip through...

changed target branch from main to dev

added 1 commit

• 89184d46 - add warning and deserializefromdict

Compare with previous version

added 1 commit

• 72b4c00b - fix docstring

Compare with previous version

Did you already test this deserializefromdict fallback with CGM? Because I am just running my version (that works without specializing to AbstractDict) through the various CGM tests.

no, have not

I implemented an alternative version over at !60 (closed) that is tested, in case we don't want to rely on narrowing down the types.

added 1 commit

• 6dce0102 - add forgotten def

Compare with previous version

added 1 commit

• 6394c31e - avoid name collision

Compare with previous version

approved this merge request

marked this merge request as ready

mentioned in commit df477093

merged