Haoti/readme update#549
Merged
hunghaoti merged 12 commits intohaoti/pre-releasev1.0.0from Dec 30, 2024
Merged
Conversation
There is a lot of duplicate code in cytnx. This patch removes some duplication of code in the file I/O
in src/Tensor.cpp, src/UniTensor.cpp, src/backend/Storage.cpp. There are sometimes 3 copies of a function, that differ only
by taking a filename as an std::string, a const char*, or an fstream. These should be consolidated to one function.
I made a start at this, and also improved some of the error messages.
A reasonable model is:
// Save(std::string) forwards to Save(const char*)
void Tensor::Save(const std::string &fname) const {
Tensor::Save(fname.c_str());
}
// Save(const char*) does some error checking and forwards to Save(fstream&)
void Tensor::Save(const char *fname) const {
fstream f;
string ffname = string(fname) + ".cytn";
f.open(ffname, ios::out | ios::trunc | ios::binary);
cytnx_error_msg(!f.is_open(), "[ERROR] invalid file path for save: %s", fname);
this->_Save(f);
f.close();
}
void Tensor::_Save(fstream &f) const {
// non-trivial code in here
}
I also improved some of the error messages, although I notice that I missed the one that I copy-pasted above.
A common pattern I see is code like this:
if (!f.is_open()) {
cytnx_error_msg(true, "[ERROR] invalid file path for save.%s", "\n");
}
I wonder if this is partly due to the name cytnx_error_msg -- it is not obvious from the name what this does,
does it unconditionally show an error message, or does it do some check, or what? I suggest to rename cytnx_error_msg to
cytnx_error_if(Condition, Message, ...)
to make it clear that it is doing a test of a condition and it does not need to be enclosed in an if (Condition) statement.
Almost every cytnx_error_msg call has "[ERROR] " at the start of the string. The few calls that do not are probably unintentional.
So a nice improvement would be to move the "[ERROR] " part of the message into the implementation of cytnx_error_xxx, so the
caller doesn't have to repeat the same string every time.
There are also a lot of cytnx_error_xxx calls that include some information about the sub-module, eg "[ERROR][Trace] invalid Type.%s". A better mechanism would be to specify the sub-module separately, for example something like
CYTNX_SUBMODULE("Trace");
which could go at the top of a C++ file (or anywhere else, really), and some macro magic would include the module string in the error message.
Also fixed a bogus change to src/UniTensor.cpp. I didn't intend to include src/UniTensor.cpp in the previous commit, it was an accident and the change was some spurious debugging stuff. That is now reverted, and took the opportunity for some simple cleanups of the error messages, to make them more uniform with Tensor.cpp
Remove some duplicate code
Improve README
1. Adjust the file to ensure the Markdown displays correctly in the GitHub viewer. 2. Add the developer and contributors. 3. Incorporate content suggested in the PR #541 . 4. Add a link to the release note. 5. Add the paper reference. 6. Remove certain content and leave them in `dox.md` file.
Closed
1. Add more description about the UniTensor and give some example code. 2. Test all code and fix. 3. Add linalg part.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Create this PR and close #547