-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update CAPI aka update to CADET v5.0 #11
Conversation
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 @Immudzen: There is an issue when reading sensitivites for one of the examples in I get the following error:
I assume it's because there is no |
As a quick fix, I did the following:
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 |
686637d
to
9331cb3
Compare
@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. 🤓 |
5ef5c28
to
ba630e0
Compare
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:
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. |
#14 is merged, I've rebased |
cfe4fe4
to
38fa4c4
Compare
Once this is merged we should merge fau-advanced-separations/CADET-Process#169 to maintain compatibility. |
warnings.warn("Deprecation warning: Support for setting cadet.cadet_path will be removed in a future version.") | ||
warnings.warn( | ||
"Support for setting cadet.cadet_path will be removed in a future version. " | ||
"TODO: What alternative should be used?", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[TODO]: @ronald-jaepel I forgot, but what alternative should be used in the future?
@hannahlanzrath Would you be up to adding the MCT in here? Edit: Since the MCT is currently not available in |
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. |
Due to the use of type hints, we now require Python >=3.10
Co-authored-by: Samuel Leweke <[email protected]>
…s in CADETMeta Class fixup! Reduce instantiation frequency of Runners by not instantiating runners in CADETMeta Class
Add `local` tag to tests that require multiple different CADET installations which is not feasible on the CI and remove those tests from the CI.
To-do
createLWE
)lrmp
should be squashed (imho)test_dll.py
(once v5.0 is released)/root/meta
time_sim
file_format
Open questions
n_units
from the input data or from the API (e.g. inCadetDLLRunner.load_solution
)?Considerations for Interface v2
Moved to cadet/CADET-Core#123