Skip to content
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

Implement GlassBR design in Python code #134

Open
11 of 12 tasks
niazim3 opened this issue Jul 24, 2018 · 16 comments
Open
11 of 12 tasks

Implement GlassBR design in Python code #134

niazim3 opened this issue Jul 24, 2018 · 16 comments
Assignees
Labels

Comments

@niazim3
Copy link
Collaborator

niazim3 commented Jul 24, 2018

As mentioned in #105; this issue is just to keep track of the progress made for the task.

Note: be sure to use doxygen-style comments for the new implementation

@smiths
Copy link
Owner

smiths commented Jul 24, 2018

Please also implement unit tests for all modules. You should use pytest, as we used in 2ME3. We should have a make rule that runs the tests.

@smiths

This comment has been minimized.

@samm82

This comment has been minimized.

@smiths

This comment has been minimized.

@niazim3
Copy link
Collaborator Author

niazim3 commented Jul 26, 2018

To Do:

Just noting here that as mentioned in the following comment ( 03fdd1b#commitcomment-29848126):

  • Tag the repo with the last version that holds the old implementation for anyone that may want to see the older implementation before it is removed
  • NewImplementation folder needs to replace Implementation folder once it's no longer helpful in the current implementation

@smiths
Copy link
Owner

smiths commented Jul 26, 2018

Sounds good.

@elwazana
Copy link
Collaborator

elwazana commented Aug 2, 2018

@smiths Hello Dr. Smith, I just had a question about the old implementation default values, did those values pass?

Old Implmentation input values:
image

@smiths
Copy link
Owner

smiths commented Aug 2, 2018

You would have to run the code to see if the input values pass. At the end of last summer all of the tests were passing, but things may have changed since then. In particular, there seems to have been a gradual move to update from mm to m. This code may have been caught in being only partially switched over.

If you are asking about mm versus m. We want to just work with m now. The input file should be changed accordingly. The only exception I can think of is for the thickness of the glass. That mm value isn't actually a measurement, but a label for the type of glass. That should stay as "mm".

@elwazana
Copy link
Collaborator

@smiths Hello Dr. Smith, I pushed the Control module as is and it completed. I've been trying to get a test for it to work and I noticed that when I set t = 5.0 or less and a and b are at their upper limit (their max value is set as 5 currently) the calculated value of q_hat becomes too big and exceeds its bounds.

Should the constraints on a and b be variable depending on the value of t? Or is something wrong with the equation?

The Control module can be found here: 81716bc

@smiths
Copy link
Owner

smiths commented Aug 20, 2018

@elwazana, can you be more specific? If a and b are 5.0, then the aspect ratio is 1.0. Is this what you mean, or do you mean the aspect ratio is 5.0? What is the input values for q_hat? What is the value of q_hat? What are the bounds for q_hat that you are referring to? Is this related to the bounds over which the interpolation is defined?

Do the calculations work for other values of inputs? There will definitely be cases where we do not have data defined for the interpolations. In these cases it is completely acceptable to inform the user that their data exceeds the allowed bounds.

@elwazana
Copy link
Collaborator

@smiths, the data constraints for a and b are given by the SRS as:
image
(Where dmin = 0.1 & dmax = 5)

The values of t (nominal thickness) are given as:
image

Jtol is calculated as such:

def calc_J_tol( params ):
    upper1 = 1
    lower1 = 1 - params.Pbtol

    upper2 = (params.a * params.b) ** (params.m - 1)
    lower2 = params.k * ((params.E * (params.h ** 2)) ** (params.m)) * params.LDF

    return (log( (log(upper1 / lower1)) * (upper2 / lower2) ))

By the equation above when a and b are large and t is small, Jtol becomes greater than its bounds mentioned here in the SRS:
image

In the Control module, when Jtol is feed in to calculate other values such as q_hat and q_hattol, an OutOfDomain error is raised.

This happens despite a, b, and t being within the constraints as seen above. Should this be the case?

@smiths
Copy link
Owner

smiths commented Aug 20, 2018

That helps. Thank you @elwazana. There is always going to be a point where the bounds for J are exceeded. It actually makes sense that having a huge plate of glass that is very thin is a bad idea. A 5 m by 5 m plate of glass (16 feet by 16 feet!) that is 5 mm thick seems ridiculous, even though I have no expertise in this field.

The constraints on a, b are not particularly educated constraints. I believe that they originally came from me and my thought that a plate larger than 5 m by 5 m seems impossible/improbable. No greater insight was involved, and no exploration of the mathematics.

We could look at the math and find a relationship between a*b and t, but we would have to assume that E, m and k are constant. (I know that they currently are, but they might not be in the future.) We could do this mathematical exercise, but we get the same information by calculating J in the program and seeing that it lies outside the bounds. You have done the important part of the analysis and shown that the reason it is outside of the bounds is that the dimensions of the glass are inadequate.

My recommendation is that the bounds on J be added to the SRS information. (I don't think they are currently in there, except implicitly through the graph you have shown above.) When J is outside of the valid range of 1 to 32, the required behaviour should be an error message that says "The value calculated for the stress distribution factor (J) lies outside the acceptable range [1, 32], likely as a result of inadequate plate dimensions."

If we make this part of the software behaviour, then your test case will now past, because the behaviour would now be what we expect.

Can you please update the SRS with this new data constraint information and requirement?

@elwazana
Copy link
Collaborator

@smiths, I made the following changes to the Data Constraints and Auxillary constants:
image

image
image

As for the requirement, R3:
image
Covers all data constraints already so I didn't add my own unless you had something else in mind for the updating the requirement as you mentioned above?

Here is the commit with the changes:
9dd83f0

@smiths
Copy link
Owner

smiths commented Aug 20, 2018

@elwazana, J is not an input, so you cannot handle it in the same way as a, b, etc. J is one of the outputs, so I suggest that you put it in Table 4 (Output Variables). Just as Pb has a constraint on the valid output values, so does J.

You are right that R3 covers the inputs, but we should add a requirement on what happens when the outputs are invalid. The wording for this new requirement would be similar to R3. Not including a requirement like this was a mistake in the original case study, since as it is now, nothing is said about what to do if Pb is out of range. Thank you for bringing up this issue. 😄

@elwazana
Copy link
Collaborator

elwazana commented Aug 20, 2018

@smiths, I corrected the mistake with the tables and created a new requirement as you mentioned:
image

For the requirement should it end with "the calculations stop" or should the program say the glass isn't safe/fails?
image

Commit with changes:
590869e

@smiths
Copy link
Owner

smiths commented Aug 29, 2018

I actually like the symbolic names Jmin and Jmax better than the hard-coded values. Jmin and Jmax should stay as auxiliary constants, and Table 4 should use these constants. I'll change the case study version of the SRS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants