-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add way for field specification to scale input data #3471
Open
ryar9534
wants to merge
10
commits into
develop
Choose a base branch
from
feature/aronson/inputScaling
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
18b8899
initial try
ryar9534 6a260f7
missed the initial condition application, but now it is doing the mul…
ryar9534 d3bfca9
Merge branch 'develop' into feature/aronson/inputScaling
ryar9534 61db5ac
fix multiple multiplications by only applying to one mesh level. work…
ryar9534 e872719
Merge branch 'feature/aronson/inputScaling' of https://github.com/GEO…
ryar9534 a51495f
Update FieldSpecificationManager.hpp
ryar9534 6c421b1
Update FieldSpecificationManager.hpp
ryar9534 679fc0c
Merge branch 'develop' into feature/aronson/inputScaling
ryar9534 3edc2de
Merge branch 'develop' into feature/aronson/inputScaling
ryar9534 e685e3c
Merge branch 'develop' into feature/aronson/inputScaling
ryar9534 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we specify an "operation type" here? So instead of
m_isScaling
specify something likem_operation
. Then we could have some enum operations. Things that come to mind are equals, addition, scaling....also...it just occurred to me that this stuff could be achieved using a function? I would think this is the better place to embed operations on fields?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, when @CusiniM and I were talking we saw that there were a few ways to do it. This just made sense to me as a starting point, so we could all hopefully look at something that works before making a final decision of how to do it. (Of course, I need to make it work first)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I do recall the discussion in Pau on this. I think we were a little bit premature about the approach of just adding a BC multiply option. Using a function to define the operation will be quite nice as we could then use a table function and apply a table based variable scalar multiplier. We could also define a random function for generating a varying scalar that follows some distribution....of course....this might be more than we want to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the table function way is nice, and thats what I recommended. I just dont know how users usually paint properties onto the mesh, and if that is easy to turn into a table. I figured I would give this a shot if it was easy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use the "functions" and add a "ScalarFunction" so the users don't have to be bothered by specifying a table when they don't need that. What do you think? @CusiniM what do you think?