Restructure residual calculation + Cstr - [opened]

[daklauss]

In GitLab by @d.klauss on Jun 25, 2024, 17:02

Merges add-dead-end-filtration -> master

TODOS:

• Move residual to module
  • Add tests
  • Add implementation for concentration
  • [>] Add implementation for viscosity (see Add residual implementation for visocisities #4)
• Add tests for CSTR (just a simple one to check all entries are combined correctly)

[daklauss]

In GitLab by @d.klauss on Jul 2, 2024, 13:58

added 3 commits

• 987fe55 - made myself confortable with pytest, added some tests and residuals
• fe80b34 - added residual and testfiles
• 88ac183 - added a smol test

Compare with previous version

[daklauss]

In GitLab by @d.klauss on Jul 3, 2024, 14:59

added 1 commit

• f1242ae - removed false risidual, + cstr residual v<0 => error

Compare with previous version

[daklauss]

In GitLab by @schmoelder on Jul 3, 2024, 17:29

Commented on CADETPythonSimulator/residual.py line 12

Please add doc strings.

[daklauss]

In GitLab by @d.klauss on Jul 3, 2024, 17:55

added 1 commit

• 8ef6753 - added some docstring

Compare with previous version

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 08:56

added 1 commit

• a5ccc36 - c should be a vector so -> np.array

Compare with previous version

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 10:06

added 1 commit

• 7c4a2f4 - improved testing residual structure + errortesting

Compare with previous version

[daklauss]

In GitLab by @schmoelder on Jul 4, 2024, 10:16

resolved all threads

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 10:51

marked the checklist item Move residual to module as completed

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 10:51

marked the checklist item Add tests as completed

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 10:51

marked the checklist item Add implementation for concentration as completed

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 10:56

added 2 commits

• a305d31 - seperated error testing in another class
• 1f9f5b2 - throw not implemented error @Viscosity

Compare with previous version

[daklauss]

In GitLab by @schmoelder on Jul 4, 2024, 12:02

Commented on CADETPythonSimulator/unit_operation.py line 577

[question] Does this automatically cast the "0" into the shape of the self.residuals['inlet']['viscosities'] array?

[alternative] Call the actual method. Raise a NotImplementedWarning and return "0" (or whatever shape is required).

[daklauss]

In GitLab by @schmoelder on Jul 4, 2024, 12:06

Commented on CADETPythonSimulator/residual.py line 4

Please add type annotations. :nerd:

(Sorry, hätte ich auch schon vorher beim Kommentar zu den docstrings schreiben können)

[daklauss]

In GitLab by @schmoelder on Jul 4, 2024, 12:06

Commented on CADETPythonSimulator/exception.py line 2

[question] Should we make this a base class for Exceptions (e.g. the NotInitializedError)?

[daklauss]

In GitLab by @schmoelder on Jul 4, 2024, 12:06

Commented on CADETPythonSimulator/unit_operation.py line 584

[formatting] Remove comments, they are now tracked in the PR (or issues)

[daklauss]

In GitLab by @schmoelder on Jul 4, 2024, 12:06

Commented on tests/test_unit_operation.py line 406

[formatting] Remove empty lines

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 13:43

Commented on CADETPythonSimulator/unit_operation.py line 577

[question] I thought that in the Ende the calculated viscosity is a property of the whole Volume. So it is a single float value?

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 16:22

Commented on CADETPythonSimulator/unit_operation.py line 577

changed this line in version 7 of the diff

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 16:22

Commented on CADETPythonSimulator/residual.py line 4

changed this line in version 7 of the diff

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 16:22

Commented on CADETPythonSimulator/unit_operation.py line 584

changed this line in version 7 of the diff

[daklauss]

In GitLab by @d.klauss on Jul 4, 2024, 16:22

added 1 commit

• d8bb803 - reworked test_unit_residual in test_unit_operation ... maybe a bit overengineered

Compare with previous version

[daklauss]

In GitLab by @schmoelder on Jul 4, 2024, 18:21

Commented on CADETPythonSimulator/residual.py line 30

[formatting] Consider splitting into several shorter lines. Python default is max 79 chars, which I think is too short. So I would suggest something like 88 chars (which is also used by black, AFAIK)

[daklauss]

In GitLab by @schmoelder on Jul 5, 2024, 09:16

Commented on CADETPythonSimulator/exception.py line 2

Should NotInitializedError also inherit?

[daklauss]

In GitLab by @schmoelder on Jul 5, 2024, 09:18

Commented on CADETPythonSimulator/exception.py line 9

[naming] There is already a NotImplementedError in the standard library. Consider renaming or even removing the exception entirely.

[daklauss]

In GitLab by @schmoelder on Jul 5, 2024, 09:19

Commented on CADETPythonSimulator/unit_operation.py line 577

True.

[daklauss]

In GitLab by @schmoelder on Jul 5, 2024, 09:19

Commented on CADETPythonSimulator/residual.py line 58

[naming] Be consistent with naming. If all the other methods use calculate_ as prefix, do the same here.

[daklauss]

In GitLab by @schmoelder on Jul 5, 2024, 09:22

We're getting there! :nerd:

I think we should add a style guide, there are still many places where there are too many (or to few) lines between imports, classes, defs etc.

• Decide on styling rules
• Consider adding a linter to your IDE.
• Consider autoformatting "on save"

I think ruff is what we need. See also #3

[daklauss]

In GitLab by @d.klauss on Jul 5, 2024, 09:49

Commented on CADETPythonSimulator/residual.py line 30

changed this line in version 8 of the diff

[daklauss]

In GitLab by @d.klauss on Jul 5, 2024, 09:49

Commented on CADETPythonSimulator/exception.py line 9

changed this line in version 8 of the diff

[daklauss]

In GitLab by @d.klauss on Jul 5, 2024, 09:49

Commented on CADETPythonSimulator/residual.py line 58

changed this line in version 8 of the diff

[daklauss]

In GitLab by @d.klauss on Jul 5, 2024, 09:49

added 1 commit

• 45b66fb - removed some lines, removed notimplementederror

Compare with previous version

[daklauss]

In GitLab by @d.klauss on Jul 9, 2024, 14:38

Commented on CADETPythonSimulator/exception.py line 2

So NotInitializedError inherits from CadetPythonSim? I guess this does make sense.

[daklauss]

In GitLab by @d.klauss on Jul 9, 2024, 14:41

added 1 commit

• 72c7af0 - Notinitialized Error now inherits from CADETPython

Compare with previous version

[daklauss]

In GitLab by @d.klauss on Jul 9, 2024, 14:43

resolved all threads

[daklauss]

In GitLab by @d.klauss on Jul 9, 2024, 14:47

Do i need to reinstall the project after alterning the pyproject.toml for ruff? And i guess we should do this in #3 ?

[daklauss]

In GitLab by @d.klauss on Jul 11, 2024, 09:16

added 1 commit

• d8fd78b - Final? test for unit_operation, fixed namening

Compare with previous version

[daklauss]

In GitLab by @d.klauss on Jul 11, 2024, 09:18

Commented on CADETPythonSimulator/residual.py line 58

yeah i changed it and forgot to fix the tests somehow .... the ci pipeline is necessary i guess