Open anthony-walker opened 8 months ago
Attention: Patch coverage is 82.24638%
with 49 lines
in your changes are missing coverage. Please review.
Project coverage is 75.74%. Comparing base (
06b3500
) to head (3193fa3
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@speth @ischoegl I am not entirely sure why the .NET
tests are failing but it doesn't seem to be with this PR, is this a known issue?
Otherwise I believe this code is ready for review.
@ischoegl these failures are from my latest bug fixes there is a separate commit which addresses the .NET
failure issue, 0a34092.
Before you get any further, I just wanted to suggest undoing the merge commit and rebasing onto the current main
branch instead.
For an example of where the current derivatives are clearly not correct, consider the following example, adapted from one of your new tests:
import cantera as ct
import numpy as np
# create first reactor
gas1 = ct.Solution("h2o2.yaml", "ohmech")
gas1.TPX = 600, ct.one_atm, "O2:1.0"
r1 = ct.IdealGasMoleReactor(gas1)
r1.name = 'R1'
gas1.set_multiplier(0.0)
# create second reactor
gas2 = ct.Solution("h2o2.yaml", "ohmech")
gas2.TPX = 300, ct.one_atm, "O2:1.0"
gas2.set_multiplier(0.0)
r2 = ct.IdealGasMoleReactor(gas2)
r2.name = 'R2'
# create wall
U = 500.0
A = 3.0
w = ct.Wall(r1, r2, U=U, A=A)
net = ct.ReactorNet([r1, r2])
def print_jac(J):
for i,j in zip(*np.nonzero(J)):
name_i = net.component_name(i).replace('temperature', 'T')
name_j = net.component_name(j).replace('temperature', 'T')
net.component_name(i).replace('temperature', 'T')
#print(f"J[{i: 3d}, {j: 3d}] = {J[i,j]:.3e}")
print(f"J[{name_i:8s}, {name_j:8s}] = {J[i,j]:.3e}")
print('Semi-analytical Jacobian:')
print_jac(net.jacobian)
print('\nFinite difference Jacobian:')
print_jac(net.finite_difference_jacobian)
which outputs:
Semi-analytical Jacobian:
J[R1: T , R1: T ] = -2.727e+00
J[R1: T , R1: H2 ] = 9.000e+05
J[R1: T , R1: H ] = 9.000e+05
J[R1: T , R1: O ] = 9.000e+05
J[R1: T , R1: O2 ] = 9.000e+05
J[R1: T , R1: OH ] = 9.000e+05
J[R1: T , R1: H2O ] = 9.000e+05
J[R1: T , R1: HO2 ] = 9.000e+05
J[R1: T , R1: H2O2] = 9.000e+05
J[R1: T , R1: AR ] = 9.000e+05
J[R1: T , R1: N2 ] = 9.000e+05
J[R1: T , R2: H2 ] = -4.500e+05
J[R1: T , R2: H ] = -4.500e+05
J[R1: T , R2: O ] = -4.500e+05
J[R1: T , R2: O2 ] = -4.500e+05
J[R1: T , R2: OH ] = -4.500e+05
J[R1: T , R2: H2O ] = -4.500e+05
J[R1: T , R2: HO2 ] = -4.500e+05
J[R1: T , R2: H2O2] = -4.500e+05
J[R1: T , R2: AR ] = -4.500e+05
J[R1: T , R2: N2 ] = -4.500e+05
J[R2: T , R1: H2 ] = 9.000e+05
J[R2: T , R1: H ] = 9.000e+05
J[R2: T , R1: O ] = 9.000e+05
J[R2: T , R1: O2 ] = 9.000e+05
J[R2: T , R1: OH ] = 9.000e+05
J[R2: T , R1: H2O ] = 9.000e+05
J[R2: T , R1: HO2 ] = 9.000e+05
J[R2: T , R1: H2O2] = 9.000e+05
J[R2: T , R1: AR ] = 9.000e+05
J[R2: T , R1: N2 ] = 9.000e+05
J[R2: T , R2: T ] = -1.887e+00
J[R2: T , R2: H2 ] = -4.500e+05
J[R2: T , R2: H ] = -4.500e+05
J[R2: T , R2: O ] = -4.500e+05
J[R2: T , R2: O2 ] = -4.500e+05
J[R2: T , R2: OH ] = -4.500e+05
J[R2: T , R2: H2O ] = -4.500e+05
J[R2: T , R2: HO2 ] = -4.500e+05
J[R2: T , R2: H2O2] = -4.500e+05
J[R2: T , R2: AR ] = -4.500e+05
J[R2: T , R2: N2 ] = -4.500e+05
Finite difference Jacobian:
J[R1: T , R1: T ] = -2.727e+00
J[R1: T , R1: O2 ] = 4.589e+04
J[R1: T , R2: T ] = 3.107e+00
J[R2: T , R1: T ] = 1.752e+00
J[R2: T , R2: T ] = -1.887e+00
J[R2: T , R2: O2 ] = -1.294e+04
@speth apologies for the delay, I have made some updates but still need to do some of the larger changes. It is still on my radar.
Changes proposed in this pull request
In this pull request I have setup basic structure for adding contributions to the jacobian from flow devices and walls. I have also added an extensible jacobian interface to the
ExtensibleReactor
system and modified the existing jacobian system to pass a vector of triplets as is needed in lieu of creating a large number of separate sparse matrices.@speth @ischoegl some additional tests are needed for the wall and flow contributions but I wanted to get the content officially into a PR.
Checklist
scons build
&scons test
) and unit tests address code coverage