Closed schumannj closed 8 months ago
Dear Dr. Schumann (@schumannj) thank you very much once again for the contributions.
I added some minor suggestions to some of the concepts added / edited. Please feel free to keep anything that looks useful.
@dalito: If you have any other comments, I thing that we can accept the changes.
@dalito @nmoust thanks a lot for your review. I will need to consider the remaining 3 comments and resolve in the next ~2 days.
@schumannj @nmoust - I proposed above (in the review comments) how we could proceed with two concepts reactor
and reaction temperature
.
Thanks @dalito! I have made some changes according to your suggestions, please double check that you can agree on the definitions now and I will resolve the conversations.
I think for now the additions are good to merge. Let me know if you notice anything else I should change.
@schumannj - I removed 7037 (reactor setup) as it's hard to define what does belong to it and what not especially for more complex reactors or plants. I also removed 7033 (set temperature). Here we need to define a group of concepts around controllers, set points, process values etc. . Defining all the combined terms "temperature setpoint", "temperature process value", "pressure setpoint", ... etc as separate concepts is probably not the way to go. Happy to discuss on this in an issue.
I also tuned some of the definitions, see diff https://github.com/nfdi4cat/voc4cat/pull/67/commits/5941f7eda4d027c42519fb2102641460544698e1
This PR and review got pretty long! It required quite some effort to review due to definitions in different areas and more intensive use of relations. Probably PRs should not get much bigger than this. Smaller (=less terms) are much easier for both the submitter and the reviewer. This is all a learning experience and I learned that we should encourage small contributions more. I assume that currently people think that one or two small additions may not be appreciated but we definitely do. @nmoust - This is also worth to mention in guidelines v2 and the next readme-update.
@schumannj - Thank you very much for all your work! Merging now...
add more vocabulary for heterogeneous catalysis (for use in NOMAD) and add references to some child IRIs