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

creates dummy file for YALMIP #77

Merged
merged 3 commits into from
Mar 17, 2023
Merged

creates dummy file for YALMIP #77

merged 3 commits into from
Mar 17, 2023

Conversation

araujoms
Copy link
Contributor

This is dummy file used by YALMIP to detect SeDuMi's version and enable support for complex numbers.

This is dummy file used by YALMIP to detect SeDuMi's version and enable support for complex numbers.
@johanlofberg
Copy link
Member

Isn't it better to call it something sensible so that you don't have to add the file dummy2 in 10 years when some other patch has to be detected

Maybe connect it to a planned release number, such as sedumi.version.1.3.6

@araujoms
Copy link
Contributor Author

good point.

@siko1056
Copy link
Member

siko1056 commented Mar 17, 2023

@araujoms Thanks for the patch. In my opinion this change introduces a maintenance burden. One file, that is certainly always forgotten to be changed before a release. Before there was a "version.txt" 671741c#diff-85d331d58b8c25c608de67c421ce1fe26dfea129a9626abe166c742aa4c075f2 that had the same problem: Nobody updated it and it caused confusion with users e.g. #38 .

@johanlofberg I haven't looked in a while into the version detection of SeDuMi inside YALMIP. Can you extract the latest version from the sedumi.m

sedumi/sedumi.m

Line 314 in 9ec11a7

my_fprintf(pars.fid,'SeDuMi 1.3.5 by AdvOL, 2005-2008 and Jos F. Sturm, 1998-2003.\n');

or the first line of Changelog.md?

I would also agree on a more straight forward way, e.g. sedumi_version.m just returning the latest version, but it should be generic and not tailored to YALMIP 😇

@araujoms
Copy link
Contributor Author

Another possibility is to not mention the version number in the file, just call it enable_complex or something. It's an ugly hack but at least it doesn't create a maintenance burden.

@johanlofberg
Copy link
Member

johanlofberg commented Mar 17, 2023

Having a file returning the version is something that could be used in YALMIP. I have not added support for such detection (it is currently based on finding unique files in a release), but I could rather easily extend the framework. It is actually something I have planned anyway

Reading a file and looking for a magic string feels more expensive and less generic

@johanlofberg
Copy link
Member

I added an experimental feature in the develop branch which allows detection by comparison with self-reported version

solver(i).detector= @()isequal('1.3.6',sedumi_version());

Don't ask me why I didn't use strcmpi...Will be cleaned up and generalized for some other solvers.

@araujoms
Copy link
Contributor Author

That doesn't seem ideal, it will make YALMP bug out when SeDuMi 1.3.7 is released.

@siko1056
Copy link
Member

siko1056 commented Mar 17, 2023

@araujoms I do not see how this approach will fail? I think going with #77 (comment) fine.

@araujoms
Copy link
Contributor Author

It's because of how solver detection in YALMIP works. Currently it looks for some files. If they are still there YALMIP will accept the new version without problems. But now Löfberg is demanding an exact match on the version number. When SeDuMi 1.3.7 is released YALMIP won't be able to detect it.

@siko1056
Copy link
Member

Sorry I looked a bit short sighted on @johanlofberg . I think once we decided for the solution here in SeDuMi, @johanlofberg can work on a clean solution for YALMIP.

Is there any objection about my latest change? Then I can merge this file, update it and make a 1.3.6 release of SeDuMi?

@araujoms
Copy link
Contributor Author

No objection from my side. It does create a maintenance burden, but you were the one opposed to it.

Maybe change sedumi.m to read the version number from this file, so that there's still a single place it needs to be updated?

@siko1056
Copy link
Member

Yes, that is what I thought as well 👍

@siko1056 siko1056 merged commit 897a804 into sqlp:master Mar 17, 2023
@araujoms araujoms deleted the patch-3 branch March 17, 2023 16:28
@johanlofberg
Copy link
Member

Yes, not sure what one should do really. It is a bit like cplex, the way to understand which version is installed is to look for cplex.x.y.z.dll, and then when they release a new version, a new yalmip update is required. Have to ponder this for a bit,

Regardless, I think sedumi should have a function which contains version data so this can be checked algoritmically by both sedumi it self and others

@araujoms
Copy link
Contributor Author

One method would be to convert the version number to a vector of doubles, and then check whether the vector is larger than some key version (under lexicographic ordering). A bit annoying to code, but it allows you to support future versions without updating YALMIP, and have different features for different versions.

@johanlofberg
Copy link
Member

Yes, I am adding a more general framework with support for self-reported versions, and required versions. Being lazy, I asked chatgpt to write the comparison of semantic versions and indeed it did the boring work for me.

@araujoms
Copy link
Contributor Author

Seriously? It actually managed to write the code? Fuck, the robot apocalypse is coming.

@johanlofberg
Copy link
Member

Yes, https://github.com/yalmip/YALMIP/blob/develop/extras/isVersionRecent.m was written without any adjustment at all by me using the prompt

write a matlab function which takes two arguments as strings which represent a semantic version x.y.z, and compares these and returns true if the first argument is more recent than the first

Btw, it should return true also when they are the same

it should be capable of comparing strings like 2.0 vs 2.0.1 also, using the assumption that 2.0 really means 2.0.0

@araujoms
Copy link
Contributor Author

I'm relieved it couldn't do it perfectly. Still, what it managed to do with the prompt is already creepy, it's what I would expect from an average student.

@johanlofberg
Copy link
Member

To be fair, it did pretty much what I asked from it, so the person to blame is not the programmer but the person who wrote the vague specification of edge cases...

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