Vector integration in the DblPend example#4223
Vector integration in the DblPend example#4223sarrasoussia wants to merge 343 commits intoCSchankGAfrom
Conversation
JacquesCarette
left a comment
There was a problem hiding this comment.
As a first step, this is good. Now on to making this not component-wise!
| import Language.Drasil.Code (quantvar) | ||
| import Drasil.DblPend.ODEs (dblPenODEInfo) | ||
|
|
||
| -- import Language.Drasil(vec2D, vAdd, vSub, vScale, norm, square) |
There was a problem hiding this comment.
Don't leave commented-out code in files. Just delete it.
| (sy angularVel_1 $* positionXEqn_1) | ||
|
|
||
| velVecEqn_2 :: PExpr | ||
| velVecEqn_2 = vec2D |
There was a problem hiding this comment.
Although this is correct, what we would really want here is a definition at the level of vectors rather than its components. We might not be able to 'say' that right now in Drasil, but we should at least 'say' it in a comment.
| (sy angularVel_1 $* positionXEqn_1) | ||
| -- Vector2 velocity | ||
| velVecExpr_2 :: PExpr | ||
| velVecExpr_2 = vec2D |
There was a problem hiding this comment.
same here (and likely everywhere in this file): the quantities should be computed 'vectorially' instead of 'component-wise'.
Yes, having everything inside a vector is an improvement, but we would really like to phrase things at the higher level.
There was a problem hiding this comment.
Could you please clarify what you meant by “vector operations now handled”? Should we be using things like vecAdd, vScale, and deriv explicitly in expressions now? Or were you mainly referring to grouping components into vector quantities like velocityVec_1? Just want to make sure I’m applying it the right way.
There was a problem hiding this comment.
Yes, exactly (vecAdd, etc).
|
Does anyone remember how our discussion back in #4247 concluded w.r.t. to this PR? |
|
I admit to being confused with where we are at with this. I know we want to have double pendulum use vectors directly, rather than just focus on the components. @sarrasoussia made changes to the SRS to rewrite the theories in terms of vectors. It is my job to review this, but I'll have to track down which PR has this version when I get time to review. Maybe it is this one? |
|
Hmm, maybe this PR is superseded by #4261? https://github.com/JacquesCarette/Drasil/pull/4261/files#diff-538bbd015d7596d8b589b6a2553f2f629687b81debba60677e7dbad9a0f88638 (give it a minute to load, but it will bring you to stable/.../DblPend_SRS.html's diff) |
4729b5a to
f85552a
Compare
ac369ab to
8174ee8
Compare
|
I believe this is now ready for a review |
|
It looks like this PR contains everything in #4261 ? Did you intend to target that PRs branch with this PR? It might be good to do that regardless, if only to review the changes unique to this PR solely within this PR. Is there any way that we can keep code generation of DblPend working? |
I wanted to check if the equations in the SRS are correct first. With the current changes, code generation is still not available. |
d26bd8f to
6afc384
Compare
|
@sarrasoussia I've gone through your new version of the SRS. A marked-up pdf is attached. The main problem is with the derivations sections. You keep mentioning Clifford algebra, but Clifford algebra should not show up at all in the SRS. Clifford algebra is internal to Drasil. Externally, we want to generate documentation that looks like typical vector algebra. Your derivations should be much like the previous derivations, except written in vector form. For the calculations (the generated code), I'm not sure really where Clifford algebra enters at all. Really, vector algebra isn't relevant in the generated code. The code is an ODE solver. The ODE can be represented as a system of equations in matrix form, but we don't do any operations on the system. (The current SRS version represents this as two separate scalar equations. We could change this to a vector representation if it is helpful, but I don't see that it adds anything right now.) The matrix representation is found by rearranging things as outlined in Dong's MEng report. Everything is then passed over to the external ODE solver library. We don't do any vector operations with these vectors for the calculations. I haven't looked at the double pendulum code in a while. Maybe we can do vector operations for the output calculations? The position vector for mass 2 is calculated using the addition of the position vector for mass 1 plus some other terms. That would give us a place to exercise the vector addition code. |
JacquesCarette
left a comment
There was a problem hiding this comment.
A lot of the more meaningful changes are very good.
There are a number of changes that are not needed and should be reverted. It does seem like this is a good set of changes (overall) to build on. Now you need to slowly re-enable that things you turned off!
| -- this is really ugly!! | ||
| -- | Read from a data description into a 'MSBlock' of 'MSStatement's. | ||
| readDataProc :: (SharedProg r) => DataDesc -> GenState [MSBlock r] | ||
| readDataProc :: (OOProg r) => DataDesc -> GenState [MSBlock r] |
There was a problem hiding this comment.
Why change SharedProg to OOProg here, what is the reason for doing that?
There was a problem hiding this comment.
That's right, unnecessary. My fault
|
|
||
| yAxisDirDesc :: Sentence | ||
| yAxisDirDesc = atStartNP (direction `the_ofThe` yAxis) `S.is` S "directed opposite to" +:+. phrase gravity | ||
| yAxisDirDesc = atStartNP (direction `the_ofThe` yAxis) `S.is` S "directed opposite to" +:+. phrase gravity No newline at end of file |
There was a problem hiding this comment.
Please don't change files that don't need changing!
There was a problem hiding this comment.
I dont know why this happened
| IScope scope, | ||
| IChar [] charsOfReader [], | ||
| IOrgSec inModel (SRS.inModel [] []) EmptyS], | ||
| IOrgSec inModel (SRS.inModel [] []) (foldlSent [ |
There was a problem hiding this comment.
These new sentences (which are fine) should not be inlined, but defined below and used here.
There was a problem hiding this comment.
I did remove them for now
| tMods = [accelerationTM, velocityTM, newtonSL] | ||
|
|
||
| --------------------------------- | ||
| ------------------------------ |
There was a problem hiding this comment.
Also try not to have spurious changes (like deleting 3 -) that distract from the meaningful changes.
| velYDerivEqn3_1 = sy yVel_1 $= neg (deriv (sy lenRod_1 $* cos (sy pendDisAngle_1)) time) | ||
| velYDerivEqn4_1 = sy yVel_1 $= neg (sy lenRod_1 $* deriv (cos (sy pendDisAngle_1)) time) | ||
| -- ===================================================== | ||
| -- Vector-based Derivations using Clifford Algebra |
There was a problem hiding this comment.
Positive comment: a lot of these are not much more vector-based than component based, and that is very good.
| import Control.Lens ((^.)) | ||
|
|
||
| -- | Helper function to create Clifford vector spaces of a given dimension | ||
| realVect :: Dimension -> Space |
There was a problem hiding this comment.
Same here, abstract out (won't repeat again)
code/stable-website/index.html
Outdated
| <ul class="list"> | ||
| <li> | ||
| DblPend - To predict the motion of a double pendulum. | ||
| DblPend - To predict the motion of a double pendulum using formulation. |
There was a problem hiding this comment.
"using formulation" is neither necessary, nor is it grammatical English. Please revert.
| using std::vector; | ||
|
|
||
| void read_table(string filename, vector<double> &z_vector, vector<vector<double>> &x_matrix, vector<vector<double>> &y_matrix) { | ||
| void read_table(string filename, vector<double> &z_vect3DSor, vector<vector<double>> &x_matrix, vector<vector<double>> &y_matrix) { |
There was a problem hiding this comment.
Does this name in the generated code really need to change?
| Successfully matched Function (String :| [Real,Real]) Real with Func [String] Double. | ||
| Successfully matched Vect Real with List Double. | ||
| Successfully matched Vect (Vect Real) with List (List Double). | ||
| Successfully matched ClifS (Fixed 3) Vector Real with List Double. |
There was a problem hiding this comment.
As Dr. Smith said on other parts already, ClifS should not escape the insides of Drasil. Best would be if this were to say (details TBD) "Vector of Length 3 of Real" (and below "3x3 Matrix ..."
| <h2>Scope of Requirements</h2> | ||
| <p class="paragraph"> | ||
| The scope of the requirements includes the analysis of a two-dimensional (<span title="two-dimensional">2D</span>) pendulum motion problem with various initial conditions. | ||
| The scope of the requirements includes the analysis of a two-dimensional (<span title="two-dimensional">2D</span>) pendulum motion problem with various initial conditions using for geometric representation of physical quantities. |
There was a problem hiding this comment.
not grammatical -- needed?
My formatter switched out the `*`'s for `-`'s, and I didn't notice until I saw the diff. I'm switching back now.
As with my other PR, my formatter automatically replaced the latter with the former, thus making diff comparison impossible. This puts it back, so the diff should be readable again.
Position vector DataDefs (p_1, p_2) caused type checking errors because ddENoRefs with PExpr cannot handle vector construction using vect[...] and cScale operators. The type checker cannot infer vector types in this context, even though the same expressions work fine in Expressions.hs. Changes: - Moved position vector expressions to Expressions.hs (mvPosExpr_1/2) - Commented out positionVecDD_1/2 from dataDefs list in DataDefs.hs - Updated GenDefs.hs to remove references to positionVecDD_1 - Updated velocity derivation sentences to not reference DataDefs Position vectors are still defined and used in: - Expressions.hs: mvPosExpr_1, mvPosExpr_2 - Unitals.hs: posVec_1, posVec_2 - GenDefs: velocity/acceleration/force derivations Type checker now passes with no errors.
Fixed the wiki link for GOOL/GProc Overview
...from sidebar to working-notes
Result of executing `find . -type f -name "*.hs" -exec perl -0777 -i -pe 's/\n\n\n/\n\n/g' {} +`.
`drasil-lang`: Switch `RawContent`'s `CodeBlock` constructor from using `CodeExpr` to `Expr`.
Updated "Getting to Know Potential Users of Drasil" Page
Wiki: Updated SubPackages to Match Its Current Role
Linter Workflow: Add check for excessive consecutive newlines and remove offending newlines.
`drasil-database`: Update READMEs.
Wiki: Debugging in Drasil
Remove `drasil-database`-related re-exports from `drasil-lang`.
|
This PR introduces a substantially complete integration of GA into the dblpend example. Currently fixing the merge conflicts |
- Updated GenDefs.hs to use QPP.mass and phrase mass - Updated Derivations.hs to use QPP.mass in all expressions - Updated Expressions.hs to use QPP.mass in all expressions - Cleaned up blank line in Unitals.hs Remaining issue: Runtime error 'Failed to find chunk mass (expected type: DefinedQuantityDict)' needs investigation into ChunkDB registration and UID preservation.
This PR was originally intended to isolate the dblpend integration work from #4261, but a few small updates to other examples were necessary to ensure the entire project builds successfully.