davidschlachter / flash-separation Goto Github PK

It's manually specified in our tests and we haven't figured out why this is needed!

i.e. a constructor that takes a FlowStream object and reads the variables from the source to create the new object. Note that deep copying is required for the FlowSpecies objects in the FlowStream's list of species.

I'm pretty sure we do some things like return this.FlowSpecies, but we should do something more like return new FlowSpecies(this.FlowSpecies) instead to prevent having the same object exist all over the place. This is related to issue #15.

Difficult to say if it actually works or not :) This issue should be closed when there's a test for each thing that the Controller actually does.

Easy opportunity for file IO — might have a user switch for extra verbosity.

Unless someone can reliably get the heat of vapourization from what was in our program before 1a9c2db, we'll need a heat of vapourization for all the PresetSpecies in order to calculate enthalpy. @Stephlanctott and I found that while you can get it from a steam table, you can't get it from integrated liquid/vapour heat capacities.

This causes compiler warnings — maybe the method should be moved into the Test_Fugacity class instead and just be a private method?

Will do this after materials quiz. Posting it as a problem so I don't forget.

Related to Scanner's processing on lines / characters / and string...

computeNonIdealParameters method in fugacity class is a good opportunity to have checks to make all of the necessary user inputs have been made in order to calculate non-ideal properties of a species

or should it access a copy?

(i.e. difference between this.flowStream = flowStream; vs this.flowStream = new FlowStream(flowStream); in the constructor)

We added some critical properties to the Species class for non-ideal calculations, but the getters and setters (with associated error checks) aren't tested.

The testAntoineCoefficients in the Test_PresetSpecies class calculates the saturation pressures at the known boiling points of all the PresetSpecies. This should give atmospheric pressure (101325 Pa) in each case. However, some species are causing the test to fail, so the units of the Antoine coefficients are not correct.

For non-ideal cases, the activity coefficient and largePhi are used in Raoult's law to correct for non-ideal gas and non-ideal liquid behaviour. Species should have ideal defaults, and relevant calculations should use the constants.

Required for dew point calculation but excluded in ideal gas mode in addCustomSpecies.

The test that I added in 7df175b from LearnChemE doesn't give an adiabatic result in our simulator.

I've converted all the units over and we get that the enthalpy change between the two states is about 3000–8000 J, while we expect it to be almost exactly zero.

I've improved the logic a bit in 1aed95d but there's more work to be done!

We're only tested the Enthalpy class with temperature changes at constant pressure, but we haven't tested changing both temperature and pressure (which is what our flash simulator requires). This needs a validated test from the steam tables!

I'm trying this right now with a mixture of water and methanol and the results are not consistent! (The two numbers in the picture should be of equal magnitude but opposite sign)

Is this necessary? The acentric factors are added manually in the non-ideal Rachford Rice test, but are they calculated by the Fugacity class or do they need to be provided? If so, we'll need them in PresetSpecies.

This needs to be implemented!

Using the getADouble() method allows us to check for errors and remove repetition (related to #3 : )

The test for calculating the ideal dew point of a mixture containing a non-condensing component is now failing — maybe this is related to the updates to the bounds finder? Ideally the fix to this wouldn't change the test — only the machinery that does the calculations.

We only have two tests but should of course cover all the expected behaviours.

Related to #4

Design-wise I think we'd need to:
- Do these calculations in Excel (for a given mixture: solve SRK equation, get K's for RR equation, solve composition)
- Add some properties to the species class (critical pressure, molecular weight) and also to the PresetSpecies list (with corresponding tests)
- Make a new class (SRK) with a (static?) method to add activity coefficients to all the FlowSpecies of a FlowStream object
- Make sure the RachfordRice class actually uses the activity coefficients when determining K values
- Add tests based on the expected results from the Excel sheets to make sure the code is doing the calculations right!

All the values that are fetched with getADouble are zero!

The upper and lower bounds of the Antoine coefficients for species in PresetSpecies mostly have 'dummy values' — we need to put the actual ones in.

Also, we need to include multiple Antoine coefficient values for all the species in PresetSpecies — I think we should start with the ones available on the NIST site.

The comments in PresetSpecies should also indicate the source of the Antoine coefficients.

Currently we just use the first set, which doesn't always make sense.

need new hand calculations

So I changed the non-ideal dew point test to compute the non-ideal parameters within the test, but our results don't agree with this SRK calculator (which does give us the same values for the Z values test in Test_Fugacity, so it seems trustworthy to me). We get 368.5 K, they get 367.9 K. We do get some error messages too in the console, and I'm not sure if they're relevant.

We should make sure that our non-ideal calculator gives the same values as some reference implementation / result.

We added some new species properties recently for non-ideal cases and they're not all in the clone method.

We need the units of heat capacities to be in J/mol with temperatures in K, since we take molar flow rates and perform calculations on a molar basis. I just changed the heat capacity values for water vapour (in da9a431), which were in J/mol, but gave a C_p/R value instead of C_p (values were taken from Table C1 in Smith & Van Ness — other values from that table will need to same adjustment). I noticed that some heat capacities given in other sources are in energy/mass instead of energy/mole, and some of this might be in our PresetSpecies as well!

We probably need to check all the heat capacity units, reference them, and make corrections as necessary!

We don't have a rigorous Excel sheet with an enthalpy calculation corresponding to a unit test. Alternatively, calculating the enthalpy change of a textbook example with a given answer would make a good test.

Steam tables give the difference in enthalpy between superheated steam at 1 atm from 125 – 150 ºC as about 899 J / mol. [1, 2]

If we do this calculation in our program, we get 12377.85 J/mol. However, manually doing the integration in Excel with the constants we have in our program gives 859.982 J/mol. Clearly our program isn't doing the right thing!

I think this is related to our assumption that a mixture is always in VLE when we perform the calculation — I think we're integrating vapour heat capacities from the bubble point to the target temperature, but for superheated vapour this doesn't work. When iterating we may need to calculate enthalpy changes for superheat or supercooled mixtures, so this should be corrected to do the right thing in both cases.

We should fix this by demonstrating a test case that successfully calculates the enthalpy change in subcooled liquid water to a higher temperature, from liquid to vapour (or to VLE), and from super-heated vapour to a higher temperature. Our program should do the right thing in all three cases! (Related to #5)

It does check the mole fractions, temperatures, and pressures, but the Antoine coefficients and vapour/liquid heat capacity constants aren't checked. We don't plan to use the approxEquals method in cases where these might be different, but the method won't give the expected result if this is the case.

Maybe we should have a list of Antoine coefficient objects stored in each Species? This way we could select the appropriate coefficients depending on the temperature.

We should remove unnecessary debug messages

Initial guess affects where Ridders Method converges on a root. May be due to too large a difference between lower and upper bounds being passed from getBounds. Will look into optimizing this.

Right now the bounds are arbitrarily set to 200 – 1000 K for dew/bubble point or 0.5 – 5.0 for the Rachford-Rice equation, but if the value is outside this range it will not be found. Also, for some mixtures the sign of the testFunction is the same at both of these boundaries, preventing the RootFinder from converging. We should have some sensible method for finding the bounds in either case.

For example, we could evaluate testFunction at the FlowStream temperature and step up until we get a change in sign.

We're missing this data for everything except for water and cyclohexane.

Constants need to be expressed such that the quadratic equation used by HeatCapacity will return values in units of J/mol, for temperatures in K. (Often constants are expressed to give C_p/R, but we need them to return C_p — see #25).

I don't know enough about the math to know whether we want 2 and 4 or 2.0 and 4.0. Here's what we have right now:

for(j = 0; j < i; j++){
          if(i>k)
            sumTerm += flowStream.getFlowSpecies().get(i).getVapourMoleFraction() * 
            flowStream.getFlowSpecies().get(j).getVapourMoleFraction() * 
            ( (4 * bij[i][k]) - bij[i][i] - (2 * bij[k][k]) - (2 * bij[i][j]) + bij[j][j]);
          else
            sumTerm += flowStream.getFlowSpecies().get(i).getVapourMoleFraction() * 
            flowStream.getFlowSpecies().get(j).getVapourMoleFraction() * 
            ( (4 * bij[k][i]) - bij[i][i] - (2 * bij[k][k]) - (2 * bij[i][j]) + bij[j][j]);
        }

The following classes should probably implement an equals method:

• FlowSpecies
• FlowStream
• Species
• AntoineCoefficients

I'm not sure how inheritance would affect the equals methods for FlowSpecies and Species (e.g. is there a call to super somewhere?).

Important principle of programming: Don't repeat yourself

The ConsoleUI class is full of pieces that should be turned into separate methods to remove repetition. Otherwise, changes to one functionality need to be made all over the class. For example, there should be a 'getInput' class that gets user input, checks if it's valid (from an array of possible answers, or a range of doubles, etc; and if invalid, asks again), and returns it to the calling method. At present this is copy-pasted every time it happens!

Also, separating the pieces into modules makes them more testable.

We currently handle most error by giving an error message and exiting with a non-zero exit code. However, it would be better (and more in line with course objectives) to use exception handling when errors occur.

If so, it should either overwrite them or throw an exception.

Fuzzers generate random input and feed to the the methods of our classes, looking for crashes and unexpected results. We should use one as part of our quality control, especially before finalizing our code (and to test other group's programs!).

Examples:

https://github.com/cphr/javafuzz
https://bitbucket.org/blob79/quickcheck
https://www.owasp.org/index.php/OWASP_JBroFuzz_Tutorial#How_to_Use_JBroFuzz_as_a_Fuzzing_Library