search

NNPDF / pineappl

PineAPPL is not an extension of APPLgrid
https://nnpdf.github.io/pineappl/
GNU General Public License v3.0
13 stars 3 forks source link

Propagate conventions into the C and Fortran APIs #311

Open Radonirinaunimi opened 2 months ago

Radonirinaunimi commented 2 months ago

The renaming conventions should be propagated into the C and Fortran APIs as well as in the examples folder.

felixhekhorn commented 2 months ago

what were you thinking about? because "convolve" went through I believe ...

cschwan commented 2 months ago

There are still lumi when it should be channel, but remember that we can't/shouldn't change the CAPI and by extension the Fortran C bindings - this would break all programs linking the CAPI. What we can and will do after merging the changes from v1-file-format, is to deprecate the lumi methods and add new function for the new functionality.

alecandido commented 2 months ago

Yes, in this sense I strongly agree with @cschwan: PineAPPL is the project where stability should be taken most seriously.

Especially, when there are third-party tools not that actively maintained around. And supporting them is a selling point of PineAPPL.

What you could do is instead a "long term" deprecation plan (as @cschwan also proposed): just extend the API with the new conventions, and mark as deprecated the old one. Still, you may be unable to actually drop the old for years, but it's a reasonable compromise.

felixhekhorn commented 2 months ago

well as said https://github.com/NNPDF/pineappl/commit/2a8a9912a29d81d8b4ca4906eeec8daf6f5f80ea broke already the interface ... I can see your point and I agree with it, but as Germans would say "the child already fell into the well" (in English this even rhymes :rofl: )

cschwan commented 2 months ago

@felixhekhorn The CAPI wasn't broken by this commit, in fact you can still use the old functions as is done in examples/cpp/deprecated.cpp.

felixhekhorn commented 2 months ago

@felixhekhorn The CAPI wasn't broken by this commit, in fact you can still use the old functions as is done in examples/cpp/deprecated.cpp.

I see ... so the commit didn't break the CAPI (which is the relevant information), but the Fortran API

cschwan commented 2 months ago

Removing the Fortran binding was probably unintentional and that will break compilation, but existing programs should not break when you just replace the compiled library. Maybe that's even a good strategy to force people to upgrade.

alecandido commented 2 months ago

Removing the Fortran binding was probably unintentional and that will break compilation, but existing programs should not break when you just replace the compiled library. Maybe that's even a good strategy to force people to upgrade.

Existing statically-compiled programs :P

(and I still consider better to adopt the deprecation plan explicitly, not to nudge users to upgrade just "breaking something, but not everything")

cschwan commented 2 months ago

Dynamically-compiled programs, actually, because the CAPI function is still there.

Radonirinaunimi commented 2 months ago

My idea was that ultimately we will have to fix the Fortran API in order to be able to use it in https://github.com/NNPDF/sihp-pp, and therefore take that opportunity to polish things and propagate the conventions.

I do understand indeed that this will have to be done with the utmost care, and I agree with @cschwan's plan.