-
Notifications
You must be signed in to change notification settings - Fork 137
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
adding sparse sensing postprocessor and associated plots #2044
Conversation
…-AddingSPSL-PP with additional commits
Job Test Ubuntu 18 PIP on 0c3300d : invalidated by @Jimmy-INL |
@joshua-cogliati-inl, this PR has passed all checks. If you can assign a task to Niharika to populate the documentation to that part and create another PR to Jimmy-INL:Jimmy-Josh-AddingSPSL-PP. Also, I tried to add a space to the 'Temperature (K)' as the label of the colorbar but it still misses the last round bracket. it labels it as |
I am not quite sure what documentation you want created? Hm, I wonder if somewhere the number of letters is being trimmed for the colorbar label? |
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.
@Jimmy-INL I have a couple of comments for you to consider. In addition, is it possible for you to reduce the size of CSV files?
\item \xmlNode{Goal}, \xmlDesc{string, required field}, the goal of the sparse sensor optimization. | ||
User has to provide \xmlAttr{subType} which is a \xmlDesc{string, required field} representing the goal of the sparse sensing optimization; i.e., which goal function is used in the optimization? Examples for such goal functions are: | ||
\begin{itemize} | ||
\item \textbf{Reconstruction} deals with predicting the values of a quantity of interest at different locations other than those where sensors are located. For example, one might predict the temperature at a point point in the middle of a fuel rod based on readings taken at various other positions. |
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.
remove duplicated 'point'?
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.
Fixed.
\item \xmlNode{features}, \xmlDesc{comma separated strings, required field}, features/inputs of the data model, i.e., locations or time stamps we should sense | ||
\item \xmlNode{target}, \xmlDesc{comma separated strings, required field}, target of data model |
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 would prefer either "feature, target" pair or "features, targets" pair.
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.
Possibly make it clear in the documentation if we only support one target (Something like "Note that only one target is allowed because ...")
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.
@Jimmy-INL How should we resolve this?
printPriority=108, | ||
descr=r"""Features/inputs of the data model""") | ||
goal.addSub(features) | ||
target = InputData.parameterInputFactory("target", contentType=InputTypes.StringType, |
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.
See my previous comment about feature/target
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 added a comment about features/target in the SparseSensing.tex
\item \xmlNode{basis} , \xmlDesc{string, optional field}, the type of basis onto which the data are projected: \xmlString{Identity},\xmlString{ SVD}, \xmlString{Random}. \default{SVD} | ||
\item \xmlNode{nModes}, \xmlDesc{integer, required field}, the number of modes used to project the data. | ||
\item \xmlNode{nSensors}, \xmlDesc{integer, required field}, the number of sensors used | ||
\item \xmlNode{optimizer}, \xmlDesc{string, optional field}, the optimizer used to find the sensors: \xmlString{QR}, \xmlString{CCQR} for unconstrained and cost constrained respectively. |
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 you update this description, only 'QR' is used inside the code right now.
features[var] = np.atleast_1d(inputDS[var].data) | ||
|
||
data = inputDS[self.sensingTarget].data | ||
## TODO: add some assertions to check the shape of the data matrix in case of steady state and time-dependent data |
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.
Can you check the checks in the "TODO"?
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.
@wangcj05, this is for the case of time-dependent data, which is not done yet. I left this as a reminder for Josh when he implements it.
I have added another assert here for the steady state case, yet I don't think it is possible to make it false.
tests/framework/PostProcessors/SparseSensing/testSPSLOptiTwist.xml
Outdated
Show resolved
Hide resolved
Job Mingw Test on 2e9a3fd : invalidated by @Jimmy-INL |
Job Mingw Test on 7a28a6c : invalidated by @joshua-cogliati-inl Diff tests/framework/InternalParallelTests/ExternalModelRay |
Job Mingw Test on 7a28a6c : invalidated by @joshua-cogliati-inl Failed tests/framework/Optimizers/SimulatedAnnealing/ExponentialSA Failed tests/framework/Optimizers/SimulatedAnnealing/CauchySA |
@Jimmy-INL @joshua-cogliati-inl It seems some of my comments have not yet addressed, could you check for that? In addition, is it possible to reduce the size of csv files? |
awk 'NR % 10 == 1' < 350_240_.csv > 350_240.csv
I pushed up a version that only kept every 10th line, so they are now smaller. Jimmy-INL@9645bd1#diff-236a401eba49b77706542311f8eb33a845715e8f234a612a6dcce980b254dd4d |
… into Jimmy-Josh-AddingSPSL-PP
Job Test qsubs sawtooth on 761af9b : invalidated by @joshua-cogliati-inl Failed tests/cluster_tests/InternalParallel/test_hybrid_model_code |
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 have reviewed it and I think it is ready for merging.
printPriority=108, | ||
descr=r"""Features/inputs of the data model""") | ||
goal.addSub(features) | ||
target = InputData.parameterInputFactory("target", contentType=InputTypes.StringType, |
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 added a comment about features/target in the SparseSensing.tex
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.
changes are good.
Job Mingw Test on 99ad427 : invalidated by @joshua-cogliati-inl Failed tests/framework/ROM/MSR/parallel |
Job Mingw Test on 99ad427 : invalidated by @joshua-cogliati-inl computer rebooted |
Job Mingw Test on 99ad427 : invalidated by @joshua-cogliati-inl failed in fetch |
checklist is good, PR can be merged. |
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
Closes #2043
What are the significant changes in functionality due to this change request?
It will inform the users of the optimal locations to place their sensors in order to reconstruct the responses of interest with high precision.
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.
<internalParallel>
to True.raven/tests/framework/user_guide
andraven/docs/workshop
) have been changed, the associated documentation must be reviewed and assured the text matches the example.