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

Missing "over area Ac" #48

Open
elwazana opened this issue Jun 4, 2018 · 16 comments
Open

Missing "over area Ac" #48

elwazana opened this issue Jun 4, 2018 · 16 comments
Assignees

Comments

@elwazana
Copy link
Collaborator

elwazana commented Jun 4, 2018

Equation&$q_C(t) = h_C (T_C - T_W(t))$\\ %what happened to over area $A_C$? [SS]

See Dr. Smith's comment on issue #33

@elwazana elwazana assigned smiths and elwazana and unassigned smiths Jun 4, 2018
@smiths
Copy link
Owner

smiths commented Jun 8, 2018

Please add the information on Ac back to the data definition. Also, please add the missing information on the connection to Newton's law of cooling and the appropriate assumptions. You will handle the heat transfer from the coil the same as the heat transfer to the PCM. We also need to add this information back to the Drasil version of the SRS.

@elwazana
Copy link
Collaborator Author

elwazana commented Jun 8, 2018

@smiths Should I continue doing this for the Drasil side for now, or should I wait for @JacquesCarette and @szymczdm response to this issue as we discussed? JacquesCarette/Drasil#578

@smiths
Copy link
Owner

smiths commented Jun 8, 2018

You should wait.

@JacquesCarette
Copy link
Collaborator

Should we assign this issue to @szymczdm or create a new one for that purpose?

@smiths
Copy link
Owner

smiths commented Jun 12, 2018

I'll start with having a look at how we can remove the need to reference Ac as a note after the equation. We should be able to change how the equations are derived/combined. We should be able to make the integration explicit to bring in the Ac this way. We can discuss this further during the "all hands" meeting tomorrow (June 12).

@smiths
Copy link
Owner

smiths commented Jun 12, 2018

I believe I have good news about the reference to Ac. We do not need to attach it to the equation, since the equation is about thermal flux. The thermal flux equation doesn't depend on the area; it acts over an area. The connection to the area is made through the derivation of GD2 (Simplified rate of change of temperature) and the use of this equation in IM1 and IM2 (Energy balances for water and PCM, respectively).

The changes to the manual version of the SWHS SRS are available at:

2106695

I've indicated the important changes using comments (\wss).

@elwazana, can you please make the changes to the Drasil code for Ac and Ap (and elsewhere) as indicated by the \wss comments? The DD1 and DD2 cases are simpler than we previously discussed, but the connection to Newton's Law of Cooling still needs to be added. From discussion with @szymczdm and @JacquesCarette, we should be able to add this information as part of the derivation. There are other examples of derivations in the documentation. This derivation will follow that patten, but it will be shorter.

We will discuss this in our meeting this afternoon, but it appears that we will need to fix most of the case studies to add back in the derivation information that was inadvertently removed.

@elwazana
Copy link
Collaborator Author

@smiths Hello Dr. Smith, I just need some clarification on what I'm changing? I've been looking back and forth between the \wss comments and the current stable files for Drasil (just as a reference point to where the appropriate code is) and some of the changes proposed in the comments don't even appear in the Stable

Example:

Equation&$q_C(t) = h_C (T_C - T_W(t))$ \wss{REMOVE - over area $A_C$}\\

swhs/docs/SRS/PCM_SRS.tex

Lines 935 to 937 in 2106695

& This equation assumes that the temperature of the coil is
constant over time (\aref{A_tcoil}) \wss{REMOVE - and that it does not vary
along the length of the coil (\aref{A_tlcoil})}\\

(This paragraph above in its entirety doesn't even appear in the description)

Current Stable:
image

@smiths
Copy link
Owner

smiths commented Jun 12, 2018

@elwazana, remember this entire issue was started by stable being incorrect. When I wrote remove in SWHS that doesn't mean that it should be removed from stable; it means it should be removed from what stable should have said. 😄 If one of my \wss comments says remove, and it isn't in stable, then you can interpret that as meaning, don't add to stable.

@elwazana
Copy link
Collaborator Author

@smiths So I'm assuming then that I should add the rest of this paragraph except the \wss Remove comment?

swhs/docs/SRS/PCM_SRS.tex

Lines 935 to 937 in 2106695

& This equation assumes that the temperature of the coil is
constant over time (\aref{A_tcoil}) \wss{REMOVE - and that it does not vary
along the length of the coil (\aref{A_tlcoil})}\\

@smiths
Copy link
Owner

smiths commented Jun 12, 2018

Correct. Your goal is to make stable (and the generated code) look like the manual version, just with the removed text not included. The assumption you have highlighted should be something that you can add to the derivation, along with the mention of Newton's Law of Cooling.

@elwazana
Copy link
Collaborator Author

@smiths By add to the derivation, do you mean the Instance Model derivation? Because DataDefs don't have a derivation section.

@smiths
Copy link
Owner

smiths commented Jun 12, 2018

We need to add a derivation section to the data definition. As I alluded to above, @JacquesCarette and @szymczdm and I discussed how to add the removed knowledge back to Drasil. The description field is now auto-generated, so it doesn't easily fit back into the description. However, we have the ability to add derivation knowledge. It isn't currently part of the data definition, but @szymczdm said there is no reason it couldn't be. Therefore, we can add the derivation to the DD so that we can recapture the deleted knowledge.

We can discuss this further in our meeting this afternoon.

@elwazana
Copy link
Collaborator Author

@smiths @szymczdm Hello, so I've looked at and analyzed how derivations are added to the instance models in swhs, just to see if I can replicate that for the Data Definitions. But the problem I ran into is that the IMods are defined as RelationConcept while the DDs are defined as Contents.

I think this will lead to an error if I try to add derivations to the DDs the same way they are added to the IMods. Should I try it and see or should I avoid this method?

@smiths
Copy link
Owner

smiths commented Jun 13, 2018

@elwazana, I'm afraid that I don't know the answer to your question. Hopefully it is a straightforward answer for @szymczdm.

@szymczdm
Copy link
Collaborator

@elwazana I think this is an issue of deprecated code more than anything. Data Defs should be constructed very similarly to IMs (except DDs are [QDefinition] instead of RelationConcept). I believe this is due to the issues with the Devi branch not being solved yet.

@smiths
Copy link
Owner

smiths commented Oct 29, 2018

In looking through open issues, I can verify that this issue should still be open. The requested changes to have the manual version of SWHS match the Drasil version have not yet happened. After a discussion with @JacquesCarette, the problem seems to be that the assumptions should be referenced in the "Notes" field of the chunks, but this is the opposite of the order in how Drasil currently defines chunks. We need to sort out referencing first (JacquesCarette/Drasil#1029) before we can reasonably close this issue.

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

No branches or pull requests

4 participants