cadet / CADET-Python

File based Python interface
BSD 3-Clause "New" or "Revised" License
19 stars 3 forks source link

Update CAPI aka update to CADET v5.0 #11

Closed schmoelder closed 2 weeks ago

schmoelder commented 1 year ago

To-do

Open questions

Considerations for Interface v2

Moved to https://github.com/cadet/CADET-Core/issues/123

schmoelder commented 1 year ago

I overhauled the CADETDll class and added missing methods. I also tried adding some tests which test different configurations that split components and/or ports.

@sleweke: Could we add an option in createLWE to allow setting multiple particle types?

@Immudzen: There is an issue when reading sensitivites for one of the examples in

https://github.com/modsim/CADET-Python/blob/ac294c2dfa02a3a42fdfd7666deb548800360d5d/cadet/cadet_dll_utils.py#L208

I get the following error:

Exception ignored on calling ctypes callback function: <function param_provider_get_string_array_item at 0x7f3d54930b80>
Traceback (most recent call last):
  File "/home/jo/code/CADET-Python/examples/../cadet/cadet_dll_utils.py", line 208, in param_provider_get_string_array_item
    reader.buffer = bytes_val
UnboundLocalError: local variable 'bytes_val' referenced before assignment

I assume it's because there is no else for the if-clauses. Here, o is array([b'COL_POROSITY'], dtype=object). Any idea how to fix this?

schmoelder commented 1 year ago

As a quick fix, I did the following:

    if n in c:
        o = c[n]
        if index == 0:
            if hasattr(o, 'encode'):
                bytes_val = o.encode('utf-8')
            elif hasattr(o, 'decode'):
                bytes_val = o
            elif hasattr(o[index], 'decode'):
                bytes_val = o[index]
        else:
            if hasattr(o[index], 'encode'):
                bytes_val = o[index].encode('utf-8')
            elif hasattr(o[index], 'decode'):
                bytes_val = o[index]

It gets the job done but it doesn't make me very happy since it's more of a patch. I think I don't fully understand what's happening here so maybe we could give it some more structure?

Also, maybe we should still include some else, even if that means an exception that's makes it easier to understand what the issue is.

schmoelder commented 8 months ago

@sleweke-bayer I worked a bit on the interface and I believe we can now read everything required for feature parity with the CLI. However, when writing tests, I realized that there are still some small issues regarding the dimensionality of the parameters (see description above). Once that's dealt with, I think we're ready to clean up and merge. :nerd_face:

schmoelder commented 8 months ago

Some mismatch with solution_bulk detected (CLI returns shape=(nTime, nCol, nRad, nComp); CAPI returns shape=(nTime, nRad, nCol, nComp))

Expanding on this a little, I was again a bit confused what convention we finally used for structuring our multi-dimensional data. The only information I could find was in the Forum but this never seems to have been applied. From my understanding, we (mostly) use this structure:

bulk: 'nTime', ('nAxialCells',) ('nRadialCells' / 'nPorts',) 'nComp'
particle_liquid: 'nTime', ('nParTypes',) ('nAxialCells',) ('nRadialCells' / 'nPorts',) ('nParShells',) 'nComp'
particle_solid: 'nTime', ('nParTypes',) ('nAxialCells',) ('nRadialCells' / 'nPorts',) ('nParShells',) 'nComp', 'nBound'
flux: 'nTime', ('nParTypes',) ('nAxialCells',) ('nRadialCells' / 'nPorts',) 'nComp'

At least with this, I was able to read get consistent data shapes when comparing CLI and CAPI results. However, the flux was not consistent and would need to be restructured in CADET-Core (see 184).

If this is indeed correct, we should also update the forum post and publish this in other places s.t. we can easily access / find it.

ronald-jaepel commented 7 months ago

https://github.com/modsim/CADET-Python/pull/14 is merged, I've rebased dll onto master to remove merge conflicts and pushed that branch to https://github.com/modsim/CADET-Python/tree/dll_rebased_onto_master to avoid force-pushes. Before merging dll into master, I suggest we merge it onto dll_rebased_onto_master and then merge from there.

ronald-jaepel commented 3 months ago

Once this is merged we should merge https://github.com/fau-advanced-separations/CADET-Process/pull/169 to maintain compatibility.

schmoelder commented 3 months ago

@hannahlanzrath Would you be up to adding the MCT in here?

Edit: Since the MCT is currently not available in createLWE, we have to wait until we update the testing structure in CADET-Core (see CADET-Core/#268).

ronald-jaepel commented 3 months ago

I'm getting this error:

Traceback (most recent call last):
  File "C:\Users\ronal\PycharmProjects\CADET-Python\cadet\cadet_dll.py", line 1697, in __del__
    self._api.deleteDriver(self._driver)
OSError: exception: access violation reading 0xFFFFFFFFFFFFFFFF

When

    def __del__(self) -> None:
        """
        Clean up the CADET driver on object deletion.
        """
        self._api.deleteDriver(self._driver)

is called. Everything else works.