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

Feature/octave bindings #19

Merged
merged 10 commits into from
May 6, 2024
Merged

Feature/octave bindings #19

merged 10 commits into from
May 6, 2024

Conversation

phlptp
Copy link
Member

@phlptp phlptp commented Feb 21, 2024

No description provided.

afisher1 and others added 4 commits August 17, 2023 16:03
and renaming buildHelicsInterface.m to buildMatlabHelicsInterface.m
also fixing cleaning up some unused python modules and cleaning up a
text formatting inconsistency.
@phlptp
Copy link
Member Author

phlptp commented Feb 21, 2024

@afisher1 is this able to be merged? or is there more you have to do on this?

@phlptp
Copy link
Member Author

phlptp commented Mar 25, 2024

@afisher1 you want to finish this up, then I can test and we can try to get it into the 3.6 release version for HELICS and remove the Octave references for that release of HELICS. Then I can do a bit of work on getting some octave tests running on CI. But I think finishing this up and getting it merged is the first step.

@phlptp
Copy link
Member Author

phlptp commented May 3, 2024

@afisher1 do we need the buildOctiveHelicsInterfaces.m file any more?

@phlptp
Copy link
Member Author

phlptp commented May 3, 2024

and what versions of octave is this likely to work for?

@afisher1
Copy link
Member

afisher1 commented May 3, 2024 via email

@phlptp
Copy link
Member Author

phlptp commented May 3, 2024

I will remove it then. I am going to try to add some tests of octave as well. If it works with 8.3, I should be able to use the newest ubuntu LTS release for the tests

@afisher1
Copy link
Member

afisher1 commented May 3, 2024

Yes it should. I suggest having a Matlab test folder and a octave test folder. You will also have an octave friendly version of the startup.m file. It's currently using a function that is not in the octave 8.3. Not sure it's in the 9.4. The function is something like loadlibrary or something like that.

@phlptp
Copy link
Member Author

phlptp commented May 6, 2024

@afisher1 This works with octave 8.4 on WIndows. Doesn't work with 9.1, which seems to have to do with a bug in mingw 13.2, not something directly in octave. There are a couple of the deprecated methods that could be removed and are generating warnings when compiling. Those will need to be removed at some point. But I think I am going to merge this and then start working on the tests. with the mex file in octave the calls are different, now much more like matlab which is probably a good thing but the tests don't work anymore. So that will take time. And I have to set up the build jobs, which is more complicated such that I didn't want to do it in this PR. The builds need to be updated as well moving to HELICS 3.6 so it gets intertangled. Best to deal with that separately. But this is great and simplifies many thing in supporting octave and matlab!

@afisher1
Copy link
Member

afisher1 commented May 6, 2024

@phlptp that is great news. FYI those deprecated warnings are occuring in the Matlab mex compilation as well. That will go away when the deprecated functions are removed from the helics.h file. The code that generates the helicMex.cpp file writes a wrapper function for everything present in helics.h.

@phlptp phlptp merged commit 0cf3ec2 into main May 6, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants