Calculator app improvements#4027
Merged
roystgnr merged 24 commits intolibMesh:develfrom Dec 12, 2024
Merged
Conversation
We might have a solution that depends on other variables or other systems.
This is necessary for compatibility if we want WrappedFunctor to work with any plain FunctionBase subclass that has an init() overload.
Yo, dawg, I heard you like Adapter patterns, so I put an Adapter pattern in our Adapter pattern so you can adapt while you adapt.
I didn't actually hit this bug, but someone else might.
We don't actually use the context here, it's just a shim
There's a bit of refactoring here from when I was trying to log performance more cleanly to diagnose the problem, and then there's a reordering of regex tests that speeds up the code 500% for me. It turns out that the std::regex library is really slow, and is going to stay slow until they decide to break ABI compatibility. I don't want to rewrite those entry tests without regex and I don't want to bring in Google's RE library or even boost::regex, so this is the best we can do for now.
This gets us another factor of 2.5 speed improvement, so we're now about 15x faster than the original read_matlab() in total. That's not perfect but it's fast enough that I don't think I'll bother to try manual char-fiddling.
Member
|
The "Test with threads" error here seems to be in an example unrelated to the calculator app: |
Member
|
The "Test complex" failure seems to be an actual issue with |
We were hitting this case with adaptivity_ex4 in our "zero_sol" code, and though that's probably not a great way for that example to calculate norms it's also not something we want to break, especially because the same bug might affect the fine-system code path too.
We're apparently not hitting this in our --enable-complex CI?
Even if we can't use these dimensions later, FEMContext still wants to build them if it sees a variable of those types, just in case it hits an Elem of those dimensions.
This must have been copied and pasted from earlier harder-to-solve examples; we're not pushing our own solver options here that need to not be overridden.
|
Job Coverage, step Generate coverage on 130f9c2 wanted to post the following: Coverage
Warnings
This comment will be updated on new commits. |
||||||||||||||||||||||||||
jwpeterson
approved these changes
Dec 12, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've been using this for various tests recently, and both the infrastructure upgrades and the app upgrades are likely to be useful to others too.