GlassBR: Minor technicality in comment in param.py

[samm82]

One line 16 of both caseStudies/CaseStudies/glass/Implementations/Python/Implementation/param.py and caseStudies/CaseStudies/glass/Implementations/Python_Simplified/Implementation/param.py, it should read "(element of ["AN", "HS", "GT"])" because of the word 'element'.

Here is the line in Python, but it is the same in Python_Simplified:

caseStudies/CaseStudies/glass/Implementations/Python/Implementation/param.py

Line 16 in a7e8172

gt: glass type (stored as a string) (element of "AN", "HS", "GT")

[smiths]

@samm82, please go ahead and make the proposed change to both param.py files. Is the same problem in the generated code for GlassBR? I'm not sure if we have comments in the generated code, but, I haven't looked.

What are your thoughts on the two implementations: Python and Python_Simplified. Based on the names, it is tempting to remove Python and just focus on the simplified code. What is the difference between the two?

[samm82]

Note: in future, should work in the caseStudies repo be done on branches? I forgot to for my current changes.

It looks like focusing on the Python_Simplified code is a good idea, as discussed in JacquesCarette/Drasil#262 . Python_Simplified (PS) has a TestDocumentation folder that Python (P) does not. Also, the tests from P are saved in PS as OldTests, in addition to its own tests. The only concern I see is that certain implementations are drastically different from P to PS - I'm not sure how many examples you need, but here are some major ones from calculations.py: (forgive me if a table is the worst way to organize this)

Diff	P	PS
calc_q	caseStudies/CaseStudies/glass/Implementations/Python/Implementation/calculations.py Lines 14 to 20 in 0179d31 def calc_q(w_array, data_sd, data_q, params): """ Interpolates q (demand from IM3) from Figure 2 using wtnt and SD """ idx, jdx, kdx, num_interp1, num_interp2 = interp.find_bounds(w_array, data_sd, params.wtnt, params.sd) q = interp.interp(idx, jdx, kdx, num_interp1, num_interp2, w_array, data_sd, data_q, params.wtnt, params.sd) return q	caseStudies/CaseStudies/glass/Implementations/Python_Simplified/Implementation/calculations.py Lines 13 to 15 in 0179d31 def calc_q_hat(q, params): q_hat = q * pow((params.a * params.b), 2) / (params.E * pow(params.h, 4)) * (1 / params.gtf) return q_hat
is_safe	caseStudies/CaseStudies/glass/Implementations/Python/Implementation/calculations.py Lines 83 to 100 in 0179d31 def is_safe(pb, lr, q, params): """ Checks if Pb < Pbtol and if LR > q (T1 and T2). If both are true, the glass is considered safe. """ if pb < params.pbtol: is_safe1 = True else: is_safe1 = False if lr > q: is_safe2 = True else: is_safe2 = False if is_safe1 == True and is_safe2 == True: safe = 'For the given input parameters, the glass is considered safe' else: safe = 'For the given input parameters, the glass is NOT considered safe' return is_safe1, is_safe2, safe	caseStudies/CaseStudies/glass/Implementations/Python_Simplified/Implementation/calculations.py Lines 40 to 52 in 0179d31 def is_safe1(pb, params): if pb < params.pbtol: is_safe1 = True else: is_safe1 = False return is_safe1 def is_safe2(lr, q): if lr > q: is_safe2 = True else: is_safe2 = False return is_safe2

[samm82]

Also, where can I find the generated code for GlassBR?

[JacquesCarette]

build/GlassBR/src, with 4 sub-directories for 4 languages.

[samm82]

There isn't a comment in the generated code; however, the input file lists the thickness as "10" instead of "10.0" as in #76

[JacquesCarette]

That seems like a bug in the input file there too. We'll need to make sure the generated code knows that input should be a String (which I was fairly sure was the case).

The problem is the generated code isn't actually compiled & tested routinely. This is partly on purpose, as that would assume all Drasil devs need to have multiple development environments on their machine, which is too much to ask for. However, the Travis builds should! I thought there even was an issue for that too.

[samm82]

@JacquesCarette It is a string - the problem is that the list of accepted glass thicknesses is: ["2.5", "2.7", "3.0", "4.0", "5.0", "6.0", "8.0", "10.0", "12.0", "16.0", "19.0", "22.0"], so "10" isn't recognized as being acceptable. It needs to be rendered as "10.0" as opposed to "10".

[JacquesCarette]

Good! So it is a pure bug in the input file.

[smiths]

@samm82, let us move to Python_Simplified. Please delete the Python folder with a note in the commit message that it is deprecated. It will stay under version control, so we can always look in the past to find it again. In looking at JacquesCarette/Drasil#262, I see that we are repeating conversations we have had in the past. Clearing out the Python folder will hopefully prevent us from going in the same circle again in the future. The issue you pointed to also shows that we expect the test cases to fail. I do not believe that they were properly updated, but @niazim3 can give you a better idea, since she worked on this issue last summer.

The simplified code was created by @palmerst last summer when he refactored the GlassBR example for code generation. Steven used the interpolation following the documentation I wrote, instead of how it was done in the original code. The first version (in the Python folder) is a straight conversion of the original Fortran code. The original Fortran unintentionally obfuscated the underlying algorithm. This made code generation almost impossible. The new code (Python simplified) shows the connection to linear interpolation more clearly.

[samm82]

@smiths I just noticed this now, but Python_Simplified doesn't have a Documentation folder, so it is missing the SRS and the MG. Should I copy the one from Python over to it?

[smiths]

No, please do not copy the SRS and the MG to the Python_Simplified folder. Unless an SRS is very poorly written, it does not change with the implementation language. (The SRS should be abstract.) The design (MG) should also be language agnostic. This is why the Documentation and Implementation folders are at the same level of the folder hierarchy. The same documentation applies for all programming language Implementations. (We also don't have a separate SRS and MG for the C implementation - and we shouldn't!) All of the implementations share the same requirements and design.

[samm82]

Right - ok, I think some changes I made to the README got undid somehow, so I had removed the Documentation reference in it previously.

[samm82]

@smiths Is this issue closed then?

[smiths]

Yes, this issue can be closed. However, the test cases still fail. There is an issue for that #70. I have assigned you this issue along with @niazim3. The two of you should discuss this together. @elwazana is also working on testing documentation. Together you may want to propose modifications to the verification template that we are implicitly solving.