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

ILDG read/write needs an overhaul #100

Open
1 task done
clarkedavida opened this issue Jul 27, 2022 · 2 comments
Open
1 task done

ILDG read/write needs an overhaul #100

clarkedavida opened this issue Jul 27, 2022 · 2 comments
Labels
bug Something isn't working design Discussion on how to implement something

Comments

@clarkedavida
Copy link
Collaborator

clarkedavida commented Jul 27, 2022

It is not written in a way that is general, readable, well-organized, or in some cases correct. For example:

  • There are parts which rather pertain to LIME packing generally and not ILDG specifically. For example there is an 8-bit message begin/end flag which showed up only in a lime_record function. These 8-bit flags are really rather properties of LIME record headers, and so it would make more sense to have a LIME class of which these are attributes. The ILDG writer/reader should then inherit from this LIME class.
  • There are very few comments, which makes it challenging to read for someone who doesn't know anything about LIME.
  • It will read in at least some kinds of invalid LIME file without error. For example these message begin/end flags are never checked.
  • It is not even clear to me how to set them correctly.
  • It will read in many invalid ILDG files without error. As far as I can tell, none of the keywords necessary for ILDG are ever checked.

Maybe a better approach would be to try to port some already existing, stable LIME code into here. One could use e.g. https://github.com/usqcd-software/c-lime, but that may be a bit overkill. But at least it would be general and correct. I would recommend to then test our results against working LIME packers.

  • For now, I am just going to get things "working", i.e. get our code to read in ILDG configs without checking their LIME layout, and to write out ILDG files that a LIME packer will not complain about. I will also add enough documentation that the next person can figure out how to implement it better.

This is not a small task. In my view it's really best suited for someone who has quite a bit of time and advanced C++ skill.

@clarkedavida clarkedavida added bug Something isn't working design Discussion on how to implement something labels Jul 27, 2022
@clarkedavida
Copy link
Collaborator Author

Alright I did my share. The rest is up to someone else.

@clarkedavida
Copy link
Collaborator Author

this is also related to #136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design Discussion on how to implement something
Projects
None yet
Development

No branches or pull requests

1 participant