Skip to content
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

Do not return coordinates for singleton dimensions #339

Merged
merged 3 commits into from
Dec 11, 2024

Conversation

schmoelder
Copy link
Contributor

@schmoelder schmoelder commented Dec 7, 2024

Fixes #334

additionally, we fix the support of the old CSTR interface (fix of #311)

Checklist

Please ensure you've completed the following tasks. Mark them with an x inside the brackets:

  • I have checked the CADET Developer Guide
  • My code follows the project's code style and RSE guidelines.
  • I have run relevant tests and verified that my changes do not break existing functionality.
  • I have updated documentation as needed.
  • I have reviewed my code for security considerations and potential vulnerabilities.
  • I have added necessary comments or descriptions for maintainers and reviewers.

To do

  • Fix C-API
  • Check all units for isParticleLumped
  • Check why cadet-cli --version returns the wrong version number
  • Fix CSTR old interface support

@schmoelder schmoelder requested a review from jbreue16 December 7, 2024 08:38
@hannahlanzrath
Copy link
Collaborator

Side-Note:
The cadet-cli for this gives

This is cadet-cli version 5.0.1 (fix_coordinates_for_singleton_dimensions branch)
Built from commit d0b7e622f5a1df5c33c1aa46d4a4327d4860cbb0

Shouldn't it already be 5.0.2?

@schmoelder
Copy link
Contributor Author

Shouldn't it already be 5.0.2?

Good point! I had also noticed it but thought that maybe this was a local issue. Should have written it down somewhere...

Copy link
Contributor

@jbreue16 jbreue16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running cadet-verification i saw that the crystllization tests are broken, chromatograohy tests still work. However that happened before the changes made on this PR.
Ill see if I can fix this and push the changes here to this branch since this should also be part of 5.0.3 :D

@jbreue16
Copy link
Contributor

jbreue16 commented Dec 9, 2024

The version info is read from git as specified in the versioninfo.cpp.in and CMakeLists.txt, maybe this was lost during the shenanigans of the last release. v5.0.3 should automatically pull the correct information

CADET-Verification works fine now

@schmoelder
Copy link
Contributor Author

yeah, if you run

git describe

it returns

v5.0.1-23-ga833ff7a

not quite sure why, though...

@jbreue16
Copy link
Contributor

jbreue16 commented Dec 9, 2024

Is it worth looking deeper into this or should we just continue with v5.0.3, assuming that this is automatically fixed ?

@hannahlanzrath
Copy link
Collaborator

I assume it is connected to the complications of the last release. Since the ´cadet-cli´ of the actual release 5.0.2 shows the correct versions, personally I would just move on.

@schmoelder
Copy link
Contributor Author

let's move on.

@schmoelder schmoelder mentioned this pull request Dec 10, 2024
1 task
@jbreue16 jbreue16 added this to the v5.0.3 milestone Dec 11, 2024
schmoelder and others added 2 commits December 11, 2024 15:12
This commit fixes the calculation of constant solid volume from the old
interface fields, which are supported for backwards compatibility.
@jbreue16 jbreue16 force-pushed the fix_coordinates_for_singleton_dimensions branch from d92db32 to 16203c3 Compare December 11, 2024 14:12
@jbreue16 jbreue16 merged commit 254041d into master Dec 11, 2024
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Inconsistencies between interface specification / CLI-interface / C-API
3 participants