Skip to content

SglPend: Broken Code Gen.#4396

Draft
balacij wants to merge 1 commit intomainfrom
brokenSglPendCG
Draft

SglPend: Broken Code Gen.#4396
balacij wants to merge 1 commit intomainfrom
brokenSglPendCG

Conversation

@balacij
Copy link
Collaborator

@balacij balacij commented Oct 7, 2025

This PR highlights some bizarre behaviour. This is not a PR that is intended to be merged anytime soon. It is a PR meant to highlight issues. Once SglPend's code generation works, then we can merge this PR (or a PR that replaces this one).

Comment on lines +19 to +31
int main(int argc, const char *argv[]) {
string filename = argv[1];
double L_rod;
double m;
double α;
double θ_p;
double θ_i;
double p_x^i;
double p_y^i;
get_input(filename, L_rod, m, α, θ_p, θ_i);
derived_values(L_rod, θ_i, p_x^i, p_y^i);
input_constraints(L_rod, θ_i);
write_output(θ_p);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Invalid variable names. Conventional C++ does not use unicode characters. Additionally '^' is an operator not permitted in the name of variables.
  2. get_inputs confused me for a second. We pass in variable names but they're automatically turned into references. I'm not sure if I'm a fan of not explicitly rendering something that indicates it is going to be passed as reference and could be mutated...
  3. The only output is one of the inputs.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sure is weird - probably ought to create 3 issues for each of those.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 1, the C++ generator ought to error out. And the solution is to have the names use the features of Drasil that let you have different renderings in code vs spec. Other examples use that already.

double θ_i;
double p_x^i;
double p_y^i;
Object[] outputs = InputParameters.get_input(filename);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outputs = Input...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is making sense now:

inputs :: [DefinedQuantityDict]
inputs = map dqdWr [lenRod, QPP.mass, QP.angularAccel, pendDisplacementAngle, initialPendAngle]

outputs :: [DefinedQuantityDict]
outputs = [dqdWr pendDisplacementAngle]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, Drasil does exactly what it was asked to do! Fancy that!

Probably pendDisplacementAngle should not be an input.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing pendDisplacementAngle from the inputs list:

sglpend: The following outputs cannot be computed: pendDisplacementAngle
Unused definitions are: pendDisplacementAngle, frequency, angularFrequency, period
Known values are: l_rod, mass, angularAcceleration, initialPendAngle, ixPos, iyPos
CallStack (from HasCallStack):
  error, called at lib/Language/Drasil/CodeSpec.hs:258:20 in drasil-code-0.1.9.0-CVigL3PxUMa78erpyMb2gY:Language.Drasil.CodeSpec

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments on the issue about the right way to fix this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialPendAngle (which corresponds to $\theta_i$) is already in the inputs list.

Note: I didn't look into this further yet.

p_x^i = (double)(outputs[0]);
p_y^i = (double)(outputs[1]);
InputParameters.input_constraints(L_rod, θ_i);
OutputFormat.write_output(θ_p);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only θ_p is used, and it doesn't even have any constraints. We don't need to generate any of the other code.

Comment on lines +19 to +31
int main(int argc, const char *argv[]) {
string filename = argv[1];
double L_rod;
double m;
double α;
double θ_p;
double θ_i;
double p_x^i;
double p_y^i;
get_input(filename, L_rod, m, α, θ_p, θ_i);
derived_values(L_rod, θ_i, p_x^i, p_y^i);
input_constraints(L_rod, θ_i);
write_output(θ_p);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sure is weird - probably ought to create 3 issues for each of those.

Comment on lines +19 to +31
int main(int argc, const char *argv[]) {
string filename = argv[1];
double L_rod;
double m;
double α;
double θ_p;
double θ_i;
double p_x^i;
double p_y^i;
get_input(filename, L_rod, m, α, θ_p, θ_i);
derived_values(L_rod, θ_i, p_x^i, p_y^i);
input_constraints(L_rod, θ_i);
write_output(θ_p);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 1, the C++ generator ought to error out. And the solution is to have the names use the features of Drasil that let you have different renderings in code vs spec. Other examples use that already.

InputParameters.get_input(filename, out L_rod, out m, out α, out θ_p, out θ_i);
InputParameters.derived_values(L_rod, θ_i, out p_x^i, out p_y^i);
InputParameters.input_constraints(L_rod, θ_i);
OutputFormat.write_output(θ_p);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least the csharp is consistent with the C++!

double θ_i;
double p_x^i;
double p_y^i;
Object[] outputs = InputParameters.get_input(filename);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, Drasil does exactly what it was asked to do! Fancy that!

Probably pendDisplacementAngle should not be an input.

@balacij balacij self-assigned this Oct 7, 2025
@smiths
Copy link
Collaborator

smiths commented Oct 10, 2025

Do single pendulum and double pendulum take advantage of sharing any code, the way that SWHS and noPCM share code?

@balacij
Copy link
Collaborator Author

balacij commented Oct 10, 2025

There is a bit of sharing in the SglPend -> DblPend direction, and some common sharing from drasil-data, similar to the other examples.

drasil-example/sglpend/lib/Drasil/SglPend/Goals.hs
13: import Drasil.DblPend.Concepts (rod)

drasil-example/sglpend/lib/Drasil/SglPend/Assumptions.hs
5:import Drasil.DblPend.Assumptions (assumpBasic)

drasil-example/sglpend/lib/Drasil/SglPend/Unitals.hs
17:import Drasil.DblPend.Concepts (rod)
18:import Drasil.DblPend.Unitals (lRod)

drasil-example/sglpend/lib/Drasil/SglPend/Body.hs
22:import Drasil.DblPend.Body (justification, externalLinkRef, charsOfReader,
26:import Drasil.DblPend.Concepts (concepts, rod)
27:import Drasil.DblPend.Requirements (nonFuncReqs)
28:import Drasil.DblPend.Unitals (acronyms)
29:import Drasil.DblPend.References (citations)

drasil-example/sglpend/lib/Drasil/SglPend/Requirements.hs
9:import Drasil.DblPend.Requirements(verifyInptValsDesc)

drasil-example/sglpend/lib/Drasil/SglPend/GenDefs.hs
32:import Drasil.DblPend.Concepts (arcLen, horizontalPos,

drasil-example/sglpend/lib/Drasil/SglPend/DataDefs.hs
15:import Drasil.DblPend.Concepts (horizontalPos, verticalPos)

It looks like it's primarily sharing English definitions and a few quantities (the .Unitals imports). Do you want me to dissect this further?

@smiths
Copy link
Collaborator

smiths commented Oct 21, 2025

@balacij, don't investigate further. You have enough on your plate right now. 😄 Thank you for starting the process of getting SglPend working properly. This might be a good task for a future summer research assistant. 😄

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.

3 participants