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

WIP: Backward compatibility #2255

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pnorbert
Copy link
Contributor

No description provided.

…k arrays, and 2.6.0 attributes which now contains long double and complex attributes. Also fixed bpls to print long double values.
@pnorbert pnorbert requested a review from chuckatkins May 14, 2020 18:28
@pnorbert
Copy link
Contributor Author

I have added a few BP3 and BP4 datasets for this backward compatibility tests. They are small so far so I guess they are okay to store here for now.

@chuckatkins
Copy link
Contributor

Even if they're small it's still a bunch of binary data I'd like to avoid and not have in the git repo. It's probably time to add git-lfs into the mix for this sort of thing. If it can wait until after the release we can add the infrastructure to the build and test workflow. I don't want to start it now since it could take a fair amount of iteration to get right.

However if you think we need it now before the release then I suppose this is okay in a limited capacity so long as we don't add any more. We can then move these to lfs after when we make the shift along with additional test data sets, although these will forever be in the git history.

I'll defer to you on that decision.

@chuckatkins
Copy link
Contributor

Related #2257

@williamfgc
Copy link
Contributor

@chuckatkins do you think reusing ADIOS2-Testing to run nightly with pre-stored files makes sense? I'd be interested to contribute to more tests (which will increase the size) as they are really important for sustainability. Thanks @pnorbert for adding the tests, they are spot on.

…dependent and fails anywhere which has a different long double representation than the x64 linux of the stored example files.
@pnorbert
Copy link
Contributor Author

These tests can move to another repo entirely if we can make sure they are executed at least before every release.

It is not urgent for this release, as it only confirms we are backward compatible (for these few files anyway)

@chuckatkins
Copy link
Contributor

chuckatkins commented May 14, 2020

These tests can move to another repo entirely if we can make sure they are executed at least before every release.

I do think they should stay in this repo and be run wilth all of our PR testing.

It is not urgent for this release, as it only confirms we are backward compatible (for these few files anyway)

If that's the case then I'd like to defer merging this and have the binary data use git-lfs via whatever we come up with for #2257

Let's still keep this open regardless though.

@NAThompson
Copy link
Contributor

Do we have a paid github account? I got hit with a charge at a 1GB using git-lfs with github.

@chuckatkins
Copy link
Contributor

Do we have a paid github account? I got hit with a charge at a 1GB using git-lfs with github.

We do not and the transfer limits were a problematic issue with using git-lfs in the past. What we'll do for adios2 is use a different host for the lfs repo (gitlab.kitware.com or code.ornl.gov if we can get it appropriately public) instead of the github lfs repo.

@pnorbert pnorbert changed the title Backward compatibility WIP: Backward compatibility May 15, 2020
@pnorbert
Copy link
Contributor Author

@chuckatkins As we are inching towards the next release, it is time to revisit this PR.

@chuckatkins chuckatkins added status: work-in-progress This issue is actively being worked on triage: low This issue is a nice-to-have if we can get to it but isn't holding anybody up. labels Mar 1, 2022
@eisenhauer eisenhauer added triage: medium This issue is important to be fixed but can currently be worked around, even if poorly and removed triage: low This issue is a nice-to-have if we can get to it but isn't holding anybody up. labels Jul 2, 2022
@eisenhauer
Copy link
Member

Given #3273, I agree that it's (probably past) time to revisit this. Raising triage level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work-in-progress This issue is actively being worked on triage: medium This issue is important to be fixed but can currently be worked around, even if poorly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants