nfdi4cat / voc4cat

A SKOS vocabulary for catalysis maintained by NFDI4Cat & friends
https://nfdi4cat.github.io/voc4cat/
Creative Commons Zero v1.0 Universal
10 stars 8 forks source link

Electrochemistry terms michael goette #55

Closed RoteKekse closed 4 months ago

RoteKekse commented 4 months ago

Hey i am still working on the terms, but since i have some questions and comments i created already a pull request.

i took definition so far mostly from here: https://echem101.gamry.com/electrochemical-terms/

when do i get ids assigned? only when the pull request is finished or do i have to set them by hand in the excel file?

further what is with qualities such as current, electric potential, current density etc. should we put them in?

dalito commented 4 months ago

The current failure is due to a detail of #49 which takes a bit of time to fix. I'll look into it later. It is not caused by your xlsx file.

when do i get ids assigned? only when the pull request is finished or do i have to set them by hand in the excel file?

Because you control your range of IDs you could already use the IDs. In voc4cat they are only after merging of the PR,

i took definition so far mostly from

What is the copyright?

dalito commented 4 months ago

@RoteKekse I fixed the problem by updating the action to a new version of voc4cat-tool (0.8.2).

You accidentally committed a temporary office file that I removed (inbox-excel-vocabs/.~lock.voc4cat.xlsx#).

My updates re-started CI and it successfully converted your xlsx to turtle files and removed the xlsx file in an auto-commit (22a4558).

If you need xlsx again to further edit your terms you can get a new xlsx file from the workflow artifacts. Click on the green check-mark left of the commit hash (def449c), then "details", then go to the summary of the job, and download the artifact at the bottom. The artifact is a zip file with re-built xlsx, RDF/turtle, and HTML-documentation.

RoteKekse commented 4 months ago

@dalito i updated from the source mainyour latest changes and then a git rebase. it seems like that the excel file is not in the artifacts anymore at least i dont find it. thezip` file doesnt contain an excel just a log file

but i think i can find the excel in an older commit, i think it might be better only delete it if the CI is run on the main branch in the main repo

RoteKekse commented 4 months ago

Hey @dalito i took the description know from here. https://knowledge.electrochem.org/ed/dict.htm# I would used links but i think these resources are not persistent. I put in most terms i saw in our lab in electrocatalysis, let me know what you think.

RoteKekse commented 4 months ago

@dalito i know there is likely a lot to discuss but this is what i am using for the electrochemistry in OER in nomad. Having term IDs for these things would be great and we would be happy to test it :).

dalito commented 4 months ago

OK. We will have a look and be not too picky about minor details (which could be improved in follow-up PRs).

dalito commented 4 months ago

The hierarchy is reversed: "final potential"(7217) has child (skos:narrower) "electric potential" (7219) - I looks like you consequently have it reversed (not just for these two concepts).

nmoust commented 4 months ago

@RoteKekse, thank you very much for your contribution. Please see below a few corrections needed, and some suggestions / comments to consider:

URI Concept Comment
voc4cat_0007202 working electrode Maybe add WE as an alternative label?
voc4cat_0007203 counter electrode Maybe add CE as an alternative label?
voc4cat_0007202 reference electrode Maybe add RE as an alternative label?
voc4cat_0007209 electrochemical impedance spectroscopy Can the definition be shortened? Maybe delete from: "Due to the small amplitude... to ... a single measurement". Also "abbreviated as" can be deleted.
voc4cat_0007223 electrochemical environment Typo in definition: elctrochemical.
voc4cat_0007228 number of cycles Typos in definition: measurment and an.
voc4cat_0007231 beaker Use becker or beker as alternative labels if used.
voc4cat_0007234 charge transport Maybe create a separate concept for electrochemical potential?
voc4cat_0007236 h cell Maybe use h-cell, H cell, and H-cell as alternative labels. Also maybe change H2 to hydrogen.
voc4cat_0007238 voltammogramm Remove extra space before Graphical.
voc4cat_0007241 initial frequency Change EIS to electrochemical impedance spectroscopy, and add a full stop.
voc4cat_0007242 final frequency Change EIS to electrochemical impedance spectroscopy, and add a full stop.
voc4cat_0007243 reversible hydrogen electrode Typo in definition: sated. Also remove Abbreviated as RHE.
voc4cat_0007244 concentration Add full stop at the end of the definition.
voc4cat_0007245 solvent Add full stop at the end of the definition.
voc4cat_0007248 rotating-disk electrode Maybe add rotating disk electrode as an alternative label.
voc4cat_0007249 electrochemical cell Maybe create separate concepts for galvanic cell and electrolytic cell?
voc4cat_0007251 open circuit voltage measurement Remove space between electro and chemical.
voc4cat_0007219 electric potential Typo in definition: point --> points.
RoteKekse commented 4 months ago

The hierarchy is reversed: "final potential"(7217) has child (skos:narrower) "electric potential" (7219) - I looks like you consequently have it reversed (not just for these two concepts).

Hey @dalito thats true i intuitively recorded the parents because then you have at most one. I am sorry will correct it.

RoteKekse commented 4 months ago

Thanks @nmoust for the feedback. I get from your feedback, that in principle it looks good?

RoteKekse commented 4 months ago

I added the comments and reversed the is a relation. i think there are still some is arelation to be added but i am not 100% sure let me know what you think :)

RoteKekse commented 4 months ago

i inclued some comments from @schumannj

RoteKekse commented 4 months ago

@dalito @nmoust is it ok to merge it for now?

dalito commented 4 months ago

Yes. Just let me know if you want to update the electrical potential definition or if I should do it directly in ttl.

@RoteKekse I pushed the update.

dalito commented 4 months ago

@RoteKekse - Thank you for the contribution and your perseverance in the review process!

RoteKekse commented 4 months ago

Thank you for doing it. This is really valuable to us. :) we will include the iri in nomad in the upcoming weeks :)