-
Notifications
You must be signed in to change notification settings - Fork 3
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
Release version v1 of the file format #118
Comments
Technically we can have different file formats at the same time by versioning also the modules:
In this way we can also easily get rid of old file format - if we decide to do that - by simply throwing away the |
Since you want to guarantee backwards compatibility, you need to have both the old and the new code inside and also the converter (which could go in |
Yet another way (no idea how that would work) is to let the |
Personally, I don't care so much about splitting crates. But if the other is not obviously the good choice, I'd stick with one for simplicity. The dependency on the older version is potentially a mess. I've read in the Go ecosystem an advise for new major releases, that was to create a completely new module (in this case a new crate), and depend on the old one. |
I think I'll first go with |
This stackoverflow answer describes how to do it: https://stackoverflow.com/a/58739076/812178. That seems pretty straightforward. |
Consider that, going this way, you might have to maintain also the older version, in case of bugs preventing some kind of applications. In my opinion, I would try to keep the version management straight, and just increase the version number in new releases. Thus, I'm more in favor of |
@alecandido those two ideas aren't exclusive. You'd write use pineappl_v0 as v0; in
In this way we'd preserve an older version in The only functionality that we need from |
For as long as it is working, that's fine. Problem is if The only room remained is if you are also going to dump an old version, or just the new one (I'd recommend the later). |
Another topic: from what you said it seems you want to keep file version and library version synced
Are you sure? Definitely, it has the advantage of being more clear. |
The changes in #118 (comment) will require rewriting the
I don't think there will be incompatible logic; the logic shouldn't change, only the internal structure and the file format.
We'll always guarantee backwards compatibility, but not forwards compatibility, so eventually you'll have to update your dependencies. But we'll only change the Rust API and add a new file format ( |
In an arbitrary version of PineAPPL (the Rust library) supporting the file format version
For each file version we just need to know the latest version of PineAPPL that supports it, and that will be stored in |
The ideas outlined in #118 (comment) have been implemented in #299 and they seem to work fine. In particular, we now have a new |
Here's what I'd like to change in a newer version of the file format,
v1
, replacing the current (v0
) one:BinInfo
andBinRemapper
intoBinLimits
. The reason for having them separate is historical only (see PineAPPL file format and backwards compatibility issues #83 (comment)) and merging them will make parts of the code much easierMmv3
intoGrid
, which means that metadata will always be present making metadata-related code much shorterOrder
's member fromu32
tou8
and add another member to support Add support for additional couplings #98Subgrid
types and keep only the ones we use (see PineAPPL file format and backwards compatibility issues #83 (comment))merge allSubgrid
types into a single moduleexport most important types into the main moduleuse bincode version 2 with varint encoding, see also item here: Add offline file-size optimisation #45 (comment)Grid
:initial_state_1
,initial_state_2
and possiblylumi_id_types
, which are important in some places; Clarify initial states and PDFs used #135 is a related Issue; done in Implement v1 file format [WIP] #299set_convolutions
withconvolutions_mut
; done in Implement v1 file format [WIP] #299SubgridParams
withExtraSubgridParams
and rename them toInterpolationParams
; make their parameterspub
instead of having many setters and getters; this has now been merged into singleInterp
typeLumiCache
toConvolutionCache
, which better reflects its purposeChannel
to allow more or less than 2 PIDs per linear combination; done in Implement v1 file format [WIP] #299Convolutions::None
Originally posted by @cschwan in #83 (comment)
The text was updated successfully, but these errors were encountered: