Skip to content

GlassBR: Move code-oriented QDefs forced into SRS to Choices as hand-wired QDefs.#4664

Draft
balacij wants to merge 6 commits intomainfrom
mvHandWiredGlassBRDefs
Draft

GlassBR: Move code-oriented QDefs forced into SRS to Choices as hand-wired QDefs.#4664
balacij wants to merge 6 commits intomainfrom
mvHandWiredGlassBRDefs

Conversation

@balacij
Copy link
Collaborator

@balacij balacij commented Jan 23, 2026

The IM and DD involving interpY and interpZ appear to be forced into the SRS as a result of not having any way to read in the quantities directly (#3355). We should be able to force them out of the SRS by moving to a list of hand-wired QDefs provided through Choices.

This PR is slightly dubious and I was a bit rough with the removal of the DD and the IM, which is why I'm leaving this PR as a draft. But feedback would be appreciated on the rationale @JacquesCarette @smiths . In the future, these models (DD/IM) can be moved towards an SDS if we don't already have a better mechanism in place for input parameters with default values AND support for matrices/lists.

@balacij balacij force-pushed the mvHandWiredGlassBRDefs branch from 76aa7e6 to 883c78f Compare January 23, 2026 04:40
@JacquesCarette
Copy link
Owner

Another PR with too much Drasil-speak in its description. And a weird side-effect of changing the order of output in the code. And I don't understand the need to move some of the code around.

But, at the higher level: if my guess of what this is doing is correct, then I think this is a change in a good direction.

@balacij
Copy link
Collaborator Author

balacij commented Jan 23, 2026

Piggybacking on #4663 (comment), this PR removes the DD+IM hack to get hand-written variable calculation solutions into the code generator by moving them to the options set we have for the code generator.

@balacij balacij force-pushed the mvHandWiredGlassBRDefs branch from dfb7999 to 834e77d Compare January 23, 2026 17:03
@balacij
Copy link
Collaborator Author

balacij commented Jan 23, 2026

Regarding the re-ordering, that just looks like it's purely by chance at the moment. The solution execution order is partially ordered about dependencies but not sorted about something else.

I tried moving around where the handWiredDefs are added to the solution search (dfb7999) but it just re-orders them again.

I think that is an issue for another PR?

@smiths
Copy link
Collaborator

smiths commented Jan 23, 2026

I think interpY and interpZ should eventually be codified as theories. Interpolation isn't part of the SRS for GlassBR, so that the SRS can remain abstract. Interpolation is part of the "how" for the SRS's "what". I believe we've talked before about the idea of refining the abstract theories of the SRS into concrete code. The interpolation is an intermediate step in this refinement. I could imagine us eventually generating documentation around these "numerical method" theories. I think the documentation for an interpolation library, would like similar to the documentation of the SRS. It would just be a matter of a viewpoint change so that interpolation becomes the "what" and the "how" would be the generated code.

@balacij balacij marked this pull request as ready for review January 24, 2026 16:35
@@ -93,8 +93,6 @@ \subsection{Table of Symbols}
\\
$\mathit{interpY}$ & InterpY & --
Copy link
Owner

Choose a reason for hiding this comment

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

Something is weird here: interpY is still in the table of symbols, but interpZ is not. This seems inconsistent: either both should be, or none should be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is now dealt with in 195f1bb. I missed an IM that also referenced interpY.

@@ -1237,9 +1232,7 @@ <h3>Data Definitions</h3>
</tr>
<tr>
<th>RefBy</th>
Copy link
Owner

Choose a reason for hiding this comment

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

This RefBy change seems not good? The "calculation of demand" is still a thing, but with this change, it has gone?

</tr>
<tr>
<th>RefBy</th>
<td>
Copy link
Owner

Choose a reason for hiding this comment

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

And here, even more content has disappeared. This content might have been "hooked in" in a sub-optimal way before -- but it shouldn't entirely disappear. It needs to been hooked in some other way.

|\\(h\\) |Minimum thickness |[DD:minThick](./SecDDs.md#DD:minThick) |\\({\text{m}}\\) |
|\\(\mathit{isSafeLR}\\) |3 second load equivalent resistance safety requirement|[IM:isSafeLR](./SecIMs.md#IM:isSafeLR) |-- |
|\\(\mathit{isSafePb}\\) |Probability of glass breakage safety requirement |[IM:isSafePb](./SecIMs.md#IM:isSafePb) |-- |
|\\(J\\) |Stress distribution factor (Function) |[IM:stressDistFac](./SecIMs.md#IM:stressDistFac)|-- |
Copy link
Owner

Choose a reason for hiding this comment

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

Was J a hack?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not entirely sure yet, but it is definitely not of type: Function!

This is the code we generate for it.

    /** \brief Calculates stress distribution factor (Function)
        \param inParams structure holding the input values
        \param q_hat dimensionless load
        \return stress distribution factor (Function)
    */
    public static double func_J(InputParameters inParams, double q_hat) {
        StreamWriter outfile;
        outfile = new StreamWriter("log.txt", true);
        outfile.WriteLine("function func_J called with inputs: {");
        outfile.Write("  inParams = ");
        outfile.Write("Instance of InputParameters object");
        outfile.WriteLine(", ");
        outfile.Write("  q_hat = ");
        outfile.WriteLine(q_hat);
        outfile.WriteLine("  }");
        outfile.Close();
        
        return Interpolation.interpZ("SDF.txt", inParams.AR, q_hat);
    }

@@ -19,31 +19,12 @@ The goal [GS:Predict-Glass-Withstands-Explosion](./SecGoalStmt.md#willBreakGS) i
|Output Constraints| |
|Equation |\\[B=\frac{k}{\left(a\\,b\right)^{m-1}}\\,\left(E\\,h^{2}\right)^{m}\\,\mathit{LDF}\\,e^{J}\\] |
|Description |<ul><li>\\(B\\) is the risk of failure (Unitless)</li><li>\\(k\\) is the surface flaw parameter (\\(\frac{\text{m}^{12}}{\text{N}^{7}}\\))</li><li>\\(a\\) is the plate length (long dimension) (\\({\text{m}}\\))</li><li>\\(b\\) is the plate width (short dimension) (\\({\text{m}}\\))</li><li>\\(m\\) is the surface flaw parameter (\\(\frac{\text{m}^{12}}{\text{N}^{7}}\\))</li><li>\\(E\\) is the modulus of elasticity of glass (\\({\text{Pa}}\\))</li><li>\\(h\\) is the minimum thickness (\\({\text{m}}\\))</li><li>\\(\mathit{LDF}\\) is the load duration factor (Unitless)</li><li>\\(J\\) is the stress distribution factor (Function) (Unitless)</li></ul>|
Copy link
Owner

Choose a reason for hiding this comment

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

Look here, J is in the description, but no longer in the notes nor does it appear as an IM. This leaves the SRS in an inconsistent state.

|Output Constraints| |
|Equation |\\[\hat{q}=\frac{q\\,\left(a\\,b\right)^{2}}{E\\,h^{4}\\,\mathit{GTF}}\\] |
|Description |<ul><li>\\(\hat{q}\\) is the dimensionless load (Unitless)</li><li>\\(q\\) is the applied load (demand) (\\({\text{Pa}}\\))</li><li>\\(a\\) is the plate length (long dimension) (\\({\text{m}}\\))</li><li>\\(b\\) is the plate width (short dimension) (\\({\text{m}}\\))</li><li>\\(E\\) is the modulus of elasticity of glass (\\({\text{Pa}}\\))</li><li>\\(h\\) is the minimum thickness (\\({\text{m}}\\))</li><li>\\(\mathit{GTF}\\) is the glass type factor (Unitless)</li></ul>|
|Notes |<ul><li>\\(q\\) is the 3 second duration equivalent pressure, as given in [DD:calofDemand](./SecDDs.md#DD:calofDemand).</li><li>\\(a\\) and \\(b\\) are the dimensions of the plate, where (\\(a\geq{}b\\)).</li><li>\\(E\\) comes from [A:standardValues](./SecAssumps.md#assumpSV).</li><li>\\(h\\) is defined in [DD:minThick](./SecDDs.md#DD:minThick) and is based on the nominal thicknesses.</li><li>\\(\mathit{GTF}\\) is defined in [DD:gTF](./SecDDs.md#DD:gTF).</li></ul> |
Copy link
Owner

Choose a reason for hiding this comment

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

Same here: q is used in the Description above, but now has gone from here.

@balacij balacij marked this pull request as draft January 26, 2026 19:46
@balacij
Copy link
Collaborator Author

balacij commented Jan 26, 2026

With this PR, I removed the IMs and the one DD involving interpZ and interpY, but with both of your comments in mind, removing them might have been the wrong thing to do.

Any thoughts on how we should fix them? They currently directly deal with symbols that directly go into code (which they shouldn't be doing).

Should we give them slightly more "abstract" definitions? e.g., $q_{tol}=f(\mathit{AR},J_{tol})$ ?

@smiths
Copy link
Collaborator

smiths commented Jan 26, 2026

With this PR, I removed the IMs and the one DD involving interpZ and interpY, but with both of your comments in mind, removing them might have been the wrong thing to do.

I agree that we don't want to remove the idea of interpolation.

Any thoughts on how we should fix them? They currently directly deal with symbols that directly go into code (which they shouldn't be doing).

Should we give them slightly more "abstract" definitions? e.g., q t o l = f ( AR , J t o l ) ?
I feel like we need a theory of interpolation. We could have a higher level theory that is abstract about the interpolating function, and refine that theory to one that uses a specific order polynomial.

I'm not sure we should do that right now though. I feel like we need to nail down exactly what we mean by theory before we start adding more theories.

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