-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New OptaDOS Photoemission Module #57
base: develop
Are you sure you want to change the base?
Conversation
Merged the elements that differed in optados_mod_working by hand to ensure functionality was retained. Untested!
Changed minor things in cell.f90 to allow the photoemission to work
The Calculation of epsilon_2 is cell volume dependent. This was changed, so that epsilon_2 uses photo_slab_volume, which should lessen the cell volume dependence of the QE values. Preliminary data suggests otherwise, so this is a WIP
Merged 'Incorrectly Commited Changes into Development Branch'
Changed the condition from equal to zero to less than tiny number for what kind of formula is used to compute the ARPES Phi angle
Implementing the use of user supplied imfp values for mixed material systems
Added statements to use the correct volume (i.e. photo_slab_volume in the case dos_per_volume during a photoemission run is enabled.
Added the option to turn off the extra output files and made it the default behaviour.
Code in dos_utils.f90 checked for uninitialised variables since they belong to other smearing methods. Changed the default efermi setting to rectify the parameters printing function. Changed printing in photo.f90 to make function calls clearer
* Create docs_to_pages * Rename docs_to_pages to docs_to_pages.yml * Update docs_to_pages.yml * Update docs_to_pages.yml * Update docs_to_pages.yml
* Create docs_to_pages * Rename docs_to_pages to docs_to_pages.yml * Update docs_to_pages.yml * Update docs_to_pages.yml * Update docs_to_pages.yml * Ran pre-commit * Update main.yml Made sure that optados.x is created to run the tests in the test-suite * Update main.yml Made sure optados.mpi is created and available for the test-suite * Update main.yml * Corrected .gitignore files * Reformulated and corrected qe_mat mpi test
@wuppersaver Thanks very much for this. I can see that you've added to the test suite, so that's really helpful. I'm back from SPL now, so hopefully can get this compiled up and tested, and will get back to you. |
Hi @ajm143 , I added the Docs to Pages task to test out the automatic making of the FORD documentation and to upload it to a GitHub pages on my personal fork. Since there is a different setup, it won't work here and I can remove it, too. How strict are you right now on the FORD annotations? I have found that the created FORD docs are not entirely correct for the photoemission module and I would have to familiarize myself with FORD a bit more to clean up the docs for the photo.f90 file. Best, |
Hi Felix (@wuppersaver), I think it makes sense for me (and probably @jryates) to review the code now, just in case there are things that we'd like changing. Ford: You're ahead of me in terms of GitHub integration. So rather than removing it, what's involved in modifying it make the docs for the official repo? As you can see, we're retrofitting FORD to the code, and I haven't gotten very far with annotating the rest of the source. However, I'd like the new code additions to be as well commented as possible -- especially as the ODG haven't written them, but might have to tweak them in the future! Best wishes, |
Hi Andrew (@ajm143 ), sounds good on the review part. I have only just started looking into the deployment of github pages and how ford interacts with it, so I will think again on how to set all things up properly and get back to you probably tomorrow. I have also thought of ways to make the user guide easier to maintain than a single latex file to copy/paste into. In the research group people have used sphinx to create documentation for their python projects, but I know that there are modules for shpinx and fortran. Given that we are using FORD already, I don't want to redo work unnecessarily, but for now I don't exactly know how to add something like a user guide to the FORD documentation that is implemented so far. One example I found is at https://stdlib.fortran-lang.org/#fortran-stdlib-api-documentation I will also start to write FORD documentation for the photoemission code, but it might take me a bit to get up to speed on this. Let me know what you think and I will get back to you with more specific instructions on the GitHub integration steps. Best wishes, Felix |
Hi Felix, It think in terms of the user guide (.tex), please just update with the photoemission info it in the user guide's current style. At a later date (version 3.0?) I'll work on how to make it interface with FORD, and probably get rid of the latex altogether. But I don't think that's for today. At the moment, if we can just get FORD comments included in the photoemission contribution and as a bonus, get the FORD it generates onto the web somewhere, I'd be satisfied. Best wishes, |
Hi Andrew - @ajm143, I have had a look and the steps should be as follows:
I have set up the action to run every time there is a new pull request or a commit to the develop branch to automatically update the documentation version on the pages. That can be changed, however. I hope that I didn't forget anything. Best wishes, Felix |
This does not compile with the nag compiler. If you don't have access to nag to test, I'm happy to recompile an updated PR. |
gfortran |
Removed extra character in formatting description
Hi Andrew, it is great to hear that only NAG had something to complain,. I got access to NAG and have changed the character that was messing with it, as well as successfully compiled the code using NAG. On the note of the pages. It could be, that the deployment of the pages is failing due to lack of writing rights. Unfortunately I don't have much experience with how to write to secured branches and don't know what kind of security you have on the different branches. I have no security on my gh-pages branch, so that might be the reason rerunning the action, hasn't worked so far. Best wishes, Felix |
Just a note: I think the Docs to Pages check is failing because its in a branch and cannot write to the webpage. I'm hoping it will be fixed when we accept the PR. If not, I'll dig further. |
Dear Felix @wuppersaver , Something we've done seems to have broken 4 of the tests after compilation. Please could you take a look? Best wishes, |
Dear Andrew @ajm143, I have checked over the tests and realised I did not update some of the benchmark files, after finding a off-by-one-error in one of the calculations during the run, that slightly changes the values. I have now updated the benchmark files and I verified that the tests pass again on my own machine. Apologies for forgetting this. Best wishes, |
Just to add to this - we have recently realised, that the way we calculate the photo-emission values within the photo.f90 module are incorrect. I have been recently working on resolving the issues here and made good progress, but there is some more work to be done. That is why I have not been able to get much further on the documentation and user documentation. The rework of those parts will require a couple of changes within the interface between photo.f90 and optics.f90 and further ones within the photo-emission module, but no major rework of the structure. If you already have some comments, I am happy to work those in to the 'correct' module. Many thanks in advance and sorry for having to backtrack slightly - this only became clear recently. |
Dear Felix @wuppersaver Ok. Thanks for letting us know. As you imply, I think we should carry on with the review, and worry about the minor changes necessary later. @jryates Are you able to help with the code review? Best wishes, |
Hi Felix (@wuppersaver) I realise we haven't made much progress at our end. Do you have anything to report from yours? Best wishes, |
This is a pull request to integrate the new photoemission module into the OptaDOS development version.