Adding OpenADAS ADF11 ccd rates#26
Conversation
ccd rates are thermal charge exchange rates between element and neutral hydrogen.
mattngc
left a comment
mattngc
left a comment
There was a problem hiding this comment.
Overall I like the direction this is going in, but there are quite a few inconsistencies with the rest of the module that need to be addressed before it can be merged. I think you must have been getting strange results in your testing because it looks like it would have been calling Recombination rates instead of CX rates.
|
I've added the core rate object and interfaces you need in PR cherab/core#87. Let me know if you need any changes. |
Names of functions changed and pointed out bugs fixed.
accommodating new structure of the thermal cx repository/data
|
I think I have gone through all the suggestions and commited changes/fixis. I am ready to get new ones!!! |
mattngc
dismissed
their stale review
Changes have been implemented. Starting a new review
mattngc
left a comment
mattngc
left a comment
There was a problem hiding this comment.
It looks good to me. Seems like all the issues in the previous review have been addressed.
There are a few minor PEP8 breakages, but they are minor so I will fix them.
I recommend making sure you test your code against PEP8.
2 participants
I just coppied what was there for other ADF11 formats and added list of ccd data into the openadas repository lists
thermalchargeexchange is bit too long name, maybe somebody could come up with some other? I included the name "thermal" so it is obvious it is between two thermalised populations of hydrogen and an element.