Add method to save Cadet sim as python file which can generate the Cadet sim again

[ronald-jaepel]

Also add test for the save_as_python method
Also changed a few lines in cadet.py to follow PEP 8

[ronald-jaepel]

This has been rebased and the tests still run. What was the reason this wasn't merged again?

[schmoelder]

Almost there...

I added docstrings and type annotations.

Moreover, I also refactored the method s.t. it not only works for Cadet objects but also more generically also for H5 types (where the method lives) or other derived classes.

[schmoelder]

I just talked to @jbreue16. He would like to test this with a simulation file that also contains output. 🤓

Maybe we need some check on the size of the parameters or we should add an argument to specify / limit the branches to be "imported".

Other than that, looks really good. Thanks for pushing it; I had already forgotten about it. 😅

[jbreue16]

ref_2DGRM3Zone_dynLin_1Comp_radZ3.zip

Writing the attached h5 with the output data to a python file using the added functionality, I observed three issues:

• long arrays are abbreviated with dots: model.root...output.solution.unit_006.solution_outlet = np.array([6.52177391e-26, 6.46449692e-26, 6.40770758e-26, ..., 4.58453836e-05, 4.54447327e-05, 4.50474730e-05], shape=(1500,))
  We should add an exception whenever that happens.

• additional dots after root: model.root...input.model.unit_000.adsorption.lin_kd = np.np.float64(1.0)

• additional np. for float64: model.root...input.model.unit_000.adsorption.lin_kd = np.np.float64(1.0)

Update: deleting one line seems to fix the second point. why was this added to the code anyways? does soemthing else break or is this required for array? UpdateUpdate: seems to be required somewhere else, needs a different fix

[ronald-jaepel]

Thanks for the additional test-case! It helped catch two extra cases that I've been able to fix two of.

* long arrays are abbreviated with dots: `model.root...output.solution.unit_006.solution_outlet = np.array([6.52177391e-26, 6.46449692e-26, 6.40770758e-26, ..., 4.58453836e-05, 4.54447327e-05, 4.50474730e-05], shape=(1500,))`
  We should add an exception whenever that happens.

Is fixed by 1416997

Additional cases from your example: On my machine it threw an error for np.array([b'EQUIDISTANT_PAR'], dtype='|S15') during the equality assertion because it couldn't handle the dtype, so I added an extra catch for that. I've also changed the way we compare arrays to use relative tolerances instead of absolute tolerances in 9df19fe

regarding:

* additional dots after root: `model.root...input.model.unit_000.adsorption.lin_kd = np.np.float64(1.0)`

I don't observe that. Is this still present in the current version of the PR for you?

Regarding:

* additional np. for float64: `model.root...input.model.unit_000.adsorption.lin_kd = np.np.float64(1.0)`

Update: deleting one line seems to fix the second point. why was this added to the code anyways? does soemthing else break or is this required for array? UpdateUpdate: seems to be rewuired somewhere else, needs a different fix

That is because numpy changed it's repr behavior in V2: https://numpy.org/devdocs/release/2.0.0-notes.html#representation-of-numpy-scalars-changed
I'll see what we can do.

[ronald-jaepel]

I can't seem to re-run the CI pipeline. I'll close this PR and re-open one to trigger the CI.

[ronald-jaepel]

Re-opening the PR helped to run the CI. All tests pass.

[ronald-jaepel]

Regarding the last open problem:

* additional np. for float64: `model.root...input.model.unit_000.adsorption.lin_kd = np.np.float64(1.0)`

That was solved with the change in 1416997 by the switch from repr(value) to np.array2string(value, ...)

[ronald-jaepel]

To summarize: I've fixed all problems that arose from Jan's test case and extended the CI test to always include these cases. I've changed the CI to use the create_LWE to create and run a simulation that we thereafter serialize and de-serialize so that we're always working with actual simulation data in our tests. Docstrings have been added. I think it's ready to be squased and merged.

[jbreue16]

Looks good to me, thanks Ron !