Closed editorialbot closed 1 year ago
editorialbot
commented
1 year ago Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.
For a list of things I can do to help you, just type:
@editorialbot commandsFor example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
@editorialbot generate pdfSoftware report:
github.com/AlDanial/cloc v 1.88 T=0.15 s (172.6 files/s, 163148.8 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
C/C++ Header 3 3307 1713 14776
C++ 4 134 246 3293
Markdown 8 98 0 331
YAML 7 74 0 273
TeX 1 11 0 120
XML 1 2 4 101
make 1 18 32 40
HTML 1 0 0 6
-------------------------------------------------------------------------------
SUM: 26 3644 1995 18940
-------------------------------------------------------------------------------
gitinspector failed to run statistical information for the repositoryWordcount for paper.md is 1220
editorialbot
commented
1 year ago Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- None
MISSING DOIs
- 10.1007/978-3-7643-7791-5_2 may be a valid DOI for title: Quaternions
- 10.1007/978-1-4419-7719-9_6 may be a valid DOI for title: Boost C++ libraries
- 10.1007/978-981-33-6781-4_6 may be a valid DOI for title: Generalization of Lattice-Based Cryptography on Hypercomplex Algebras
- 10.1007/bfb0054868 may be a valid DOI for title: NTRU: A ring-based public key cryptosystem
- 10.1109/cads.2010.5623536 may be a valid DOI for title: OTRU: A non-associative and high speed public key cryptosystem
INVALID DOIs
- None:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
AngryMaciek
commented
1 year ago Thanks for the help: @vissarion, @ludopulles,
Please take a look at this comment: https://github.com/openjournals/joss-reviews/issues/5159#issuecomment-1433252665 It should be quite relevant and useful here.
Kind Regards, Maciek
vissarion
commented
1 year ago 
ludopulles
commented
1 year ago 
danielskatz
commented
1 year ago 👋 @olexandr-konovalov - How is this review coming along?
AngryMaciek
commented
1 year ago @editorialbot commands
editorialbot
commented
1 year ago Hello @AngryMaciek, here are the things you can ask me to do:
# List all available commands
@editorialbot commands
# Get a list of all editors's GitHub handles
@editorialbot list editors
# Check the references of the paper for missing DOIs
@editorialbot check references
# Perform checks on the repository
@editorialbot check repository
# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist
# Set a value for branch
@editorialbot set joss-paper as branch
# Generates the pdf paper
@editorialbot generate pdf
# Generates a LaTeX preprint file
@editorialbot generate preprint
# Get a link to the complete list of reviewers
@editorialbot list reviewers
@olexandr-konovalov I finished my review.
@AngryMaciek There is only one issue left to resolve, see https://github.com/AngryMaciek/hypercomplex/issues/38 regarding paper references. After resolving this I will check out the last point in my review checklist and recommend acceptance!
AngryMaciek
commented
1 year ago @editorialbot check references
editorialbot
commented
1 year ago Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1007/978-3-7643-7791-5_2 is OK
- 10.1007/978-1-4419-7719-9_6 is OK
- 10.1007/978-981-33-6781-4_6 is OK
- 10.1007/BFb0054868 is OK
- 10.1109/CADS.2010.5623536 is OK
MISSING DOIs
- None
INVALID DOIs
- None@olexandr-konovalov @vissarion @ludopulles I am done on my end; waiting for further suggestions :)
AngryMaciek
commented
1 year ago Dear @ludopulles, I am writing a proposal to submit in the middle of May, it would be nice to include this work on my resume; could you please take a look at the repository in the upcoming days? Kind Regards, Maciek
ludopulles
commented
1 year ago It's a cool repository and the code definitely adds useful tools to experiment with hypercomplex algebras.
However, I don't see this repository to have the substantial scholarly effort for the JOSS. Not to be disrespectful for the many hours put in, but there are no clear signals of effort in my opinion. Regarding some of the signals in the guidelines:
Still, the repository is a nice tool to play around with the hypercomplex algebras. It could still be useful to perhaps integrate this into the Boost library in a way that it is compatible with or supersedes the Quaternion and Octonion implementations.
AngryMaciek
commented
1 year ago Dear @ludopulles,
To my understanding a "substantial scholarly effort" could be interpreted as:
The following repository excels at both aspects.
The commit history length seems to be about 3 months with a large gap in between.
This is a statement of fact, not an argument. Besides: number & timing of commits over time is not a reliable estimate of effort put into a project. JOSS itself states in the reviewer guidelines:
While some authors contribute openly and accrue a long and rich commit history before submitting, others may upload their software to GitHub shortly before submitting their JOSS paper.
As I have mentioned in my other post this is a resubmission of my previous work, which I have now elevated to a higher level. For the initial three months I have developed a general framework to operate on arbitrary large hypercomplex numbers from the Cayley-Dickson algebras. During the previous revision we agreed that the library is in a sufficient technical state for JOSS but lacks a clear real-world application, which I agreed to implement. We ended the revision, I privated the repository and went back to the development and refactoring.
I have spent last three years (on-and-off, of course) studying cryptography and investigating post-quantum methods; specifically: lattice-based cryptography which NTRU and its variants are a part of. Implementing a generalised version of these systems in high dimensional algebras is not a trivial task. Operations related to inverting a polynomial in a modular quotient ring as well as inverting a whole hypercomplex number constructed from such polynomials required an implementation from scratch. Eventually, due to the complexity of the whole procedure I have first developed the whole cryptographic part in SageMath, as it supports some algebraic operations out of the box. Only after verifying the cryptosystems, the code was reimplemented to C++ and included into the library (a necessary step: Sage calculations are too slow for such applications). These parts are reflected in the Polynomial.hpp file as well as RingInverse methods in the main class. The most important functionality of the whole library is provided by PUBLICKEY, ENCRYPT and DECRYPT functions which are very short and simple exactly because they make use of properly overloaded operators and other pre-defined functions. Such a modular structure allowed for careful unit testing at each single step (see this file) and achieving 100% code coverage. All this was very tedious, but absolutely necessary to ensure mathematical correctness for calculations which could not possibly be traced any other way (ex: multiplication in a $1024$-dimensional algebra of polynomials modulo $x^{37}-1$).
I believe that the whole general framework (which I already had three years ago) combined with the design and manual implementation of everything related to quotient rings in C++, all tests as well as examples included in the manuscript (but also see here) which present novel cryptographic results (more on that below) are enough effort for a publication in the Journal of Open Source Software.
The core 2 files in the hypercomplex directory contain <2000 loc and there is much boilerplate code
Please refer to the metrics as reported by the whedon bot, that is the official cloc configuration we should stick to. Granted: the repository contains Catch2 framework which should be excluded (that is: C/C++ Header | 13617, manually checked). Therefore I present a corrected table below:
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
C/C++ Header 1159
C++ 4 134 246 3293
Markdown 8 98 0 331
YAML 7 74 0 273
TeX 1 11 0 120
XML 1 2 4 101
make 1 18 32 40
HTML 1 0 0 6
-------------------------------------------------------------------------------
SUM: 5323
-------------------------------------------------------------------------------That is ~5k pure code lines in the repository. The core files are indeed <2k, however, it is not only the core that matters. This is a Journal of Open Source Software after all, and that is why other files with mechanisms ensuring proper software engineering standards (more on them later) should be considered just as well. I presume the whedon bot is aware of that and that's why the reporting is configured the way it as (above). On top of that, if I recall correctly, previous JOSS guidelines stated that submissions below 1k should be flagged for inspection (not rejected, flagged). With that information I believe there is no ground to argue the validity of this submission based on the line count.
- There are multiple wrappers for mpfr in C++, cf. https://www.mpfr.org/ § Interfaces for MPFR. Users can use their own interface and this makes this specification useless.
MPFR is a C library which operates on its own specific structures representing numbers of arbitrary precision. Almost all of operations specified in the template Hypercomplex class differ in their implementation one cannot manipulate MPFR variables just as any others (use of built-in methods like: mpfr_set_zero, mpfr_init2, mpfr_add is required). In order to construct hypercomplex numbers with arbitrary-high precision this class specialisation is absolutely necessary.
Yes, there are existing C++ interfaces, but please notice what they say at the official website you have linked to:
The following C++ interfaces for MPFR, very different in their design (and in particular, in the strategies for intermediate precisions, so that they can yield different results), are available:
Hypercomplex. And most users would not question the interface, they would probably conclude that Hypercomplex is crap and move on. I'd prefer to avoid such situations.The specialization to Hypercomplex seems to be exactly the same as the generic instance, except that the "norm" function is changed to the square (algebraic) norm, something which the generic case could also handle.
I understand you mean Polynomial class here. Yes, here you are right: this specialisation is almost identical to the generic version with the exception that I need to explicitly forbid e^H, norm and / operators. But on the other hand I needed to add CenterLift and RingInverse and norm2 functions which would operate specifically on Hypercomplex<Polynomial> instances and not on any others. Because of these minor adjustments I decided it would be cleaner to write out a full class specialisation in this case too. That is my personal choice.
Lastly, although the papers (QTRU, OTRU, STRU) point out there is exciting research done on hypercomplex algebras, these 3 papers were of theoretic kind and didn't have much of experiments.
Exactly. This is probably the strongest point in advantage of the following library. Publications which present novel cryptographic findings are purely theoretical. Meaning - there is no way for others to re-analyse, expand on or validate one's claims. There are just separate groups of people developing their own solutions and presenting just the final, usually unverifiable methods. This is the exact opposite of Open Science / Open Source. I believe that we ( as a community of researchers investigating cryptography) could greatly benefit from a high-quality reference point for lattice-based cryptosystems based on NTRU variants.
What is more, this is not just a hypothetical possibility. Errors really started to arise while I looked through the publications closely. In the current cryptographic literature there are two relatively recent papers which refer to STRU: Thakur and Tripathi and Singh et al. The latter (more recent) states the following:
There is an article, namely, “STRU: A Non-Alternative and Multi-dimensional Public Key Cryptosystem” given by Thakur and Tripathi in 2017 [15]. They proposed an STRU cryptosystem based on sedenions, but in the decryption process, they used associativity directly. Since we know that sedenions are non-associative, we cannot use associativity directly, as Thakur et al. did. So, their scheme does not follow the sedenionic requirements.
Therefore claims of the former duo are incorrect. What is worse: it is quite possible the paper by Singh et al. contains errors too. Section 4: "Extending the property to polynomials" does not look right. I tried to reproduce their results with Hypercomplex but with no success. Finally, I asked a question on Cryptography Stack Exchange and my doubts were confirmed. It seems that both already published papers are flawed. This could possibly have been avoided if only the authors or reviewers were aware of another piece of research that provides a stable implementation which would help them verify the original claims.
In my humble opinion: the above alone is a significant scholarly effort (as in: qualifiable novel value introduced to the research field).
It seems the number of interested people in the crypto community is limited, and thus I would say with the current state it is unlikely someone will cite this repository in their work.
I disagree. As soon as we are done here I plan to contact authors of all above mentioned publications (and possibly other researchers working on X-TRU variants) and present them this library. Hopefully it will aid them in their further work and let them validate their claims before submitting a paper. There are little more embarrassing feelings then admitting tu publish a mistake or worse: retracting a whole publication. I don't really see what are the basis for the comment above.
For machine learning, I did find a paper using hypercomplex algebras (released after the repository's initial code of 2021) but this used an own implementation. As I am not an ML expert, I can't say how likely this repository is to be cited by ML researchers.
The fact that these ML researchers needed to develop their own custom code for hypercomplex operations signals a lack of other available solutions (ex: a well-established library). Hypercomplex aims to fill this gap so that others (ex: ML specialists) would not need to develop algebraic operations on their own but rather focus on their domain. That's the core principle of open source: we are all building great things from smaller building blocks provided by others. No need to everyone to develop custom solutions to the same problem over and over.
It's a cool repository and the code definitely adds useful tools to experiment with hypercomplex algebras.
Still, the repository is a nice tool to play around with the hypercomplex algebras. It could still be useful to perhaps integrate this into the Boost library in a way that it is compatible with or supersedes the Quaternion and Octonion implementations.
The following submission is not a "tool to play around" but rather a state-of-the-art C++ implementation of an advanced post-quantum family of cryptosystems. First of its kind, as there in no other alternative which would allow to construct cryptosystems on such large algebras. That is most possibly because up to this date no one have ever proved that the X-TRU systems are able to function for sedonions (and higher) under the condition of specific choice of parameters and the initial value of $F$. That is no trivial statement as higher algebras loose crucial properties (associativity, commutativity, alternativity) and a generalisation into arbitrary large structures could just as well turn out to be impossible. I consider this a substantial result for the whole field of cryptography. Moreover, this is not just a "theoretical" publication which other researchers could trust on don't; the following repository is the first one to provide a working implementation and examples of encryption/decryption of full images (Fig.1) - hard to argue with a code anyone can run for themselves. The importance of presented work is quite substantial. Both in terms of scientific software for the community of computational mathematicians as well as conclusions for cryptographers.
Finally, why JOSS is a suitable journal for this work:
As much as I present some novel results this is still a C++ library brought to a very high standard. Realising the lack of any other similar solution to analyse/verify proposed mechanisms for lattice-based systems I decided to fill this gap and provide a reference point for other researchers in the field of cryptography. In my opinion it is more valuable for the community as a whole to have one solid and stable solution on which we all can rely on then to have one more "theoretical" paper presenting fomulas.
That is also why I ensured the repository sticks to the highest standards of scientific software engineering:
doxygen engine.make, that is to lower the energy barrier for contributions.Taking into account all of the points above I consider it quite brave to state that the following submission lacks "substantial scholarly effort".
Following the JOSS guidelines:
The decision on scholarly effort is ultimately one made by JOSS editors.
Thus I ask for the opinions of @olexandr-konovalov and @danielskatz
Kind Regards, Maciek
danielskatz
commented
1 year ago As the track editor here, I believe this meets JOSS's requirement for substantially scholarly effort.
AngryMaciek
commented
1 year ago Dear @ludopulles,
It would still help me a lot if I could include the work in my publication list.
Could you please resume your revision as soon as possible?
Kind Regards,
Maciek
ludopulles
commented
1 year ago @danielskatz @olexandr-konovalov @AngryMaciek I have finished my review and do not have other comments. I recommending acceptance.
AngryMaciek
commented
1 year ago @danielskatz @olexandr-konovalov I believe we are finished with the revision here!
olexandr-konovalov
commented
1 year ago @AngryMaciek sorry for being slow with replies. Many thanks @ludopulles and @vissarion for the thorough review, and to @AngryMaciek for the detailed answer. I also agree that the paper justifies the substantially scholarly effort requirement. What next?
When the reviewers are satisfied with the improvements, we ask that they confirm their recommendation to accept the submission. @ludopulles did this explicitly, but I can't see @vissarion confirming that - could you do this please? I see you completed checklist, but e.g. are you happy with the response on the issues you have submitted to https://github.com/AngryMaciek/hypercomplex/issues?
@AngryMaciek When a submission is ready to be accepted, we ask that the authors issue a new tagged release of the software (if changed - that seems to be the case), and archive it (I'd recommend Zenodo as explained here, and alternatives mentioned at https://joss.readthedocs.io/en/latest/editing.html#after-reviewers-recommend-acceptance). Please do this and post the version number and archive DOI in this review issue.
olexandr-konovalov
commented
1 year ago @editorialbot generate pdf
editorialbot
commented
1 year ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
vissarion
commented
1 year ago @olexandr-konovalov thanks for the ping, I though my comment https://github.com/openjournals/joss-reviews/issues/5272#issuecomment-1517745199 was enough, but provably it was not, sorry for that.
I finished with my review, my checklist and all the issues raised during this process are resolved. Therefore, I recommend acceptance of this paper!
olexandr-konovalov
commented
1 year ago @vissarion it was depending on the issue to be resolved, so it's useful to confirm it - thanks again!
AngryMaciek
commented
1 year ago Info for @olexandr-konovalov:
Version 2.0.1 : https://github.com/AngryMaciek/hypercomplex/releases/tag/v2.0.1
zenodo: https://zenodo.org/record/7937280
DOI: 10.5281/zenodo.7937280
olexandr-konovalov
commented
1 year ago @AngryMaciek thank you! Ideally Zenodo record should say
Related identifiers: Supplement to https://github.com/AngryMaciek/hypercomplex/tree/v2.0.1
instead of
Related identifiers: Derived from https://github.com/AngryMaciek/hypercomplex
to point to a specific release of the software (you can check recently published JOSS papers to see examples of this). Did you use GitHub-Zenodo integration, or uploaded the archive manually? Perhaps you can update the newly created Zenodo record to point to the precise tag? Thanks!
AngryMaciek
commented
1 year ago @olexandr-konovalov : I upload to zenodo manually; OK, I added one more identifier to the record.
olexandr-konovalov
commented
1 year ago @AngryMaciek Thanks - looks good to me. If you want to have a preview (e.g. like https://doi.org/10.5281/zenodo.7676801) you can upload .zip instead of .tar.gz though (when you set up automated GitHub-Zenodo integration, it does it for you, and will seamlessly work for all future releases).
AngryMaciek
commented
1 year ago Thanks, I'm good, let's just finish this.
olexandr-konovalov
commented
1 year ago @AngryMaciek I've done proofreading - please see https://github.com/AngryMaciek/hypercomplex/pull/44 for my suggestions. In addition, there are some notes about the references:
AngryMaciek
commented
1 year ago @olexandr-konovalov : thank you for reading over the text in detail;
I have merged your branch "as was" and corrected the references according to the above; I could not get a proper capitalization for dickson, regardless if I used a space of a dash in the bib file it always came out parsed as lowercase (parsing engine bug?) In the end I wrote CayleyDickson as this is closest to the truth.
I have generated a next release 2.0.2 and a new version on zenodo:
https://doi.org/10.5281/zenodo.7938707
DOI: 10.5281/zenodo.7938707
olexandr-konovalov
commented
1 year ago @editorialbot generate pdf
editorialbot
commented
1 year ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
olexandr-konovalov
commented
1 year ago @AngryMaciek you need to use extra {..} to enforce capitalisation, e.g. {Cayley {D}ickson} or even {{Cayley Dickson}}- and some other entries in the bibliography would benefit from this too, otherwise it says c++ instead of C++.
AngryMaciek
commented
1 year ago @olexandr-konovalov : Done!
olexandr-konovalov
commented
1 year ago @AngryMaciek thanks - checking now
olexandr-konovalov
commented
1 year ago @editorialbot generate pdf
editorialbot
commented
1 year ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
olexandr-konovalov
commented
1 year ago @AngryMaciek excellent, thanks! Since the paper is part of the master branch of the repository, of course the cleanest way would be to make a new release with THIS version of the paper - would you be able to do this, and then I will proceed with the rest of the TODO list at https://joss.readthedocs.io/en/latest/editing.html#after-reviewers-recommend-acceptance ?
AngryMaciek
commented
1 year ago Alright, give me 5 min plz, I will come back with a new one...
AngryMaciek
commented
1 year ago 2.0.3: https://doi.org/10.5281/zenodo.7938823
DOI: 10.5281/zenodo.7938823
@olexandr-konovalov
danielskatz
commented
1 year ago Technically, we don't need the paper to be in the release at all... Many authors put it in a branch that isn't part of the release. We archive the paper separately once it is published.
but having it in the release is fine as well.
olexandr-konovalov
commented
1 year ago True - I assumed it's author's choice, but the instructions allow both options. I'd put it in a separate branch if I would be writing a paper.
Anyhow, perhaps for now this is perfect, we don't need yet another release! In the future, you would be able to move it away from your master branch. Thanks @AngryMaciek for responding to all suggestions quickly!
olexandr-konovalov
commented
1 year ago @editorialbot set 10.5281/zenodo.7938823 as archive
editorialbot
commented
1 year ago Done! archive is now 10.5281/zenodo.7938823
olexandr-konovalov
commented
1 year ago @editorialbot set v2.0.3 as version
editorialbot
commented
1 year ago Done! version is now v2.0.3
Submitting author: !--author-handle-->@AngryMaciek<!--end-author-handle-- (Maciej Bak) Repository: https://github.com/AngryMaciek/hypercomplex Branch with paper.md (empty if default branch): Version: v2.0.3 Editor: !--editor-->@olexandr-konovalov<!--end-editor-- Reviewers: @vissarion, @ludopulles Archive: 10.5281/zenodo.7938823
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@vissarion & @ludopulles, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @olexandr-konovalov know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Checklists
📝 Checklist for @vissarion
📝 Checklist for @ludopulles