-
Notifications
You must be signed in to change notification settings - Fork 12
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
Refactor as a package #72
Comments
I have not had a chance to think deeply on this but my initial reaction is that I would prefer not to make this a package. I think it will add maintenance burden while providing a relatively small increase in convenience. I don't particularly want this repo to be a dependency to other packages and the spirit of the repo is to be just a collection of scripts, rather than some modular infrastructure. To achieve the objective of your "Example", my expectation would be that you use this repo as a reference for a new implementation/package that is specifically designed for benchmarking AD in ADNLPModels. Some notes about the draft PR. I do not have a clear understanding of Julia |
Respectfully, I strongly disagree. What's great about the work you (and others) did here is that now other people shouldn't have to redo it. If we need to go to the repo, download it, then copy-paste individual files and change multiple lines every time we need to run benchmarks, I'm not sure anyone will bother besides you. A functional API to create the model/run the benchmarks is much easier for scientists who don't want to dive into how models are encoded in each optimization framework. This PR doesn't come from nowhere, it comes from my desire to use your benchmarking code for a research project, and my realization that it would be much more convenient with a package. Essentially, #73 replaces the manual Git cloning with automated management through Julia's Pkg.
It probably won't be a dependency of registered, widely used packages, but it could be a dependency for a lot of academic code. Making
Extensions are a key feature of the package manager, introduced in Julia 1.9. They are critical here because they would allow users of If you are interested in reading the documentation on extensions, I will be happy to answer any questions you have.
That is easily fixed (by me), if there is interest on your part.
Ultimately it is a question of choosing programming paradigms. I would argue that modularity is never a bad thing, and that most Julia projects should be packages anyway, even if are not meant for wide reuse. This is also the workflow we recommend in https://modernjuliaworkflows.org/. Packages are the right way to deal with dependencies and versioning. |
I'd like to hear @odow's take on this |
To add some background information, @gdalle and I are currently doing just this for a benchmark in an upcoming paper of ours. We have to provide written |
I see both arguments. I'll note that the license is very permissive: https://github.com/lanl-ansi/rosetta-opf/blob/main/LICENSE.md |
I didn't mean that the main obstacle to copy pasting would be the license, my considerations were more tied to practicality and ease of installation |
This was not intended to be a benchmark. Like its namesake, rosetta code, it is first and foremost a tool for folks to compare, contrast and learn how different Julia optimization modeling layers work. It seems that you have found it to be a useful benchmark as well. That is great! But that is not my goal for this repo. For example, this repo does not include the larger network data case files, which are required for doing an OPF benchmark study. I have my own separate collections of scripts for OPF benchmarking, those are not included in this repo. If I was going to make an OPF benchmark repo, it would surely be engineered much differently than this one.
I agree this brings some convenience, I just believe that the added maintenance burden does not sufficiently offset that convenience. I am on the hook for doing this maintenance, I have to be realistic about my personal bandwith to do this.
Package version clashes are a challenge faced by this repo. This is an interesting idea to help resolve this. How do these extensions interact with package requirements as they are specified in the Project.toml file?
Correct, I don't want to have to maintain versioning convention for this repo. That adds maintenance overhead. My feeling is still that you want this repo to be a benchmark framework, where I want it to be a learning tool. Especially if you are working on a paper about optimization methods benchmarking, I think it would be best for you to make a different repo, maintained by you, that can be use for the reproducibility of that paper. |
My mistake. I heard about this repo through your Discourse post, and though you specify at first that it is only an "example implementation", the rest of the conversation focuses heavily on benchmarks. Whether you like it or not, it seems that people want to use your code for benchmarking ^^ The only question is whether they can
If you're open to that, I can also set up a pipeline with DataDeps.jl or DataToolkit that downloads the files from pglib-opf on-the-fly when the user asks for a specific instance. I've done this before for other projects, it's no trouble at all.
I understand your concern, but I don't understand where the maintenance burden would come from. Moving code from a script to an extension is basically free, and it is a one-off operation which I have already done for you. As for your lack of familiarity with extensions, they are already ubiquitous throughout the ecosystem, so accepting them may be the only way forward for productive Julia development. More importantly, if you agree to make this a reusable package instead of a collection of scripts, more people might chime in to contribute. That's exactly what I am doing right now, because I believe your work to be a very valuable resource and I want to help spread the word.
The package requirements are still specified in the
Why duplicate efforts if I can contribute to this repository instead? You know a lot more about power flow models than I do, and perhaps I'm slightly more experienced in Julia package development. We could help each other out. |
Here's my thoughts:
The second motivation for making it a package is dependency management.
A third complication is that Carleton wants to benchmark with HSL, which is not open-source. Currently, his branch sets additional options. I'd propose something like:
with usage something like
where each |
As for your comparison, you're missing the installation step, which is the main reason I did all this in the first place. Without #73, if you want reproducible experiments in your paper, you have to tell the reader to do something like git clone [email protected]:lanl-ansi/rosetta-opf.git
cd rosetta-opf
git checkout 496d02671eb123368928b8e384e84f900db7cc3c
cd ..
git clone [email protected]:power-grid-lib/pglib-opf.git
cd pglib-opf
git checkout dc6be4b2f85ca0e776952ec22cbd4c22396ea5a3
cd .. whereas #73 automates this (including the versioning) through Julia's package manager. The latest version of the PR also downloads the Of course it was just a first POC, if you change your mind about this we can add more options to the function (like solver choice or maximum solve time).
That's precisely what package extensions can help with, because you can have an environment with mandatory and optional parts. I don't think endless debate is very productive at this point. My suggestion arose from trying to actually use your code in a research setting, and realizing how much easier it would be with a package. Has anyone else beyond you ever tried the same? |
Hi @ccoffrin!
Would you be open to a PR refactoring
rosetta-opf
as a proper Julia package? That way people trying to use it don't have to copy-paste the code or download individual files.Example: what if I want to benchmark ADNLPModels with an AD backend different from the default, or with a solver different from Ipopt? At the moment I have to copy-paste and modify the
nlpmodels.jl
file at the root of this repo.The text was updated successfully, but these errors were encountered: