Skip to content

Comments

Interface package#50

Draft
KnutAM wants to merge 12 commits intokimauth:mainfrom
KnutAM:kam/MaterialModelsBase
Draft

Interface package#50
KnutAM wants to merge 12 commits intokimauth:mainfrom
KnutAM:kam/MaterialModelsBase

Conversation

@KnutAM
Copy link
Collaborator

@KnutAM KnutAM commented Jul 6, 2022

Since we are considering using a model from this package in existing code using the MaterialModelsBase.jl package, I looked into what changes would be necessary for this to work. In case you find this interesting...

Most of the interface is based on this repo, so changes are rather minimal.
The three main differences are

  • cache is a positional argument instead of a keyword arg (to enable dispatch on cache)
  • from the stress iterations, there is an additional full dimension strain tensor output (but that requires no change to codes not wanting this output), and it also accepts full dimension strain inputs as initial guesses in addition to the reduced dimension strain input.
  • optional outputs are included via an extra output argument (after cache) that can be mutated. This gives dispatch on this type, which gives "zero" cost if the supplied NoExtraOutput() type is used.

Currently, the traits part from this package has not been included - but I think that could belong in the base package as it is indifferent to the internal material model implementation.

Let me know if this is interesting to adopt - would be great to have your input on the interface to make it more general/easier to adopt.
(Note, most of the commits are just to make CI work for an unregistered package:))

@codecov-commenter
Copy link

Codecov Report

Merging #50 (27fbdc3) into main (d323aac) will increase coverage by 1.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   90.34%   91.43%   +1.09%     
==========================================
  Files          12       10       -2     
  Lines         352      292      -60     
==========================================
- Hits          318      267      -51     
+ Misses         34       25       -9     
Impacted Files Coverage Δ
src/CrystalViscoPlastic/CrystalViscoPlastic.jl 96.07% <ø> (ø)
src/FiniteStrain/neohook.jl 100.00% <ø> (ø)
src/FiniteStrain/stvenant.jl 95.23% <ø> (ø)
src/FiniteStrain/yeoh.jl 95.65% <ø> (ø)
src/Plastic.jl 98.38% <ø> (ø)
src/LinearElastic.jl 93.33% <100.00%> (ø)
src/traits.jl 61.29% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d323aac...27fbdc3. Read the comment docs.

@fredrikekre
Copy link

Is there a reason the base package can't just be included here? (Sorry if we discussed this before, I don't remember.)

@KnutAM
Copy link
Collaborator Author

KnutAM commented Jul 6, 2022

Is there a reason the base package can't just be included here? (Sorry if we discussed this before, I don't remember.)

The main idea is a lightweight package to depend on, which doesn't care how material models are implemented but rather focuses on what can be done with a material model. E.g. when writing an FE-program, constitutive driver, or doing material parameter identification. Hence leaving packages like this free to include additional dependencies, be less restrictive about exported names, and not include all features supported by the interface (e.g. differentiation wrt. parameters).

From my side would be fine to include the additional features here, but I assumed that was out of scope for this package...

@lijas
Copy link
Collaborator

lijas commented Jul 7, 2022

I see your point of havning one package with the interface, and another package with the implementations of the material models. But I dont think the separation is really needed in this case, and think they can be in the same package.

cache is a positional argument instead of a keyword arg (to enable dispatch on cache)

Nice

optional outputs are included via an extra output argument (after cache) that can be mutated.

This is something I wanted as well. I tried something similar in this PR, but instead returned the extra output as a 4th output instead (instead of having a mutated object). I dont know which option is best though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants