remake dopedxpy-sc-fermi

[alexsquires]

• rename bulk_vasprun, not clear that this needs to be for a high quality dos
• use _format_defect_name() in examples (is this still a private function? If it's externally useful, should it be changed?).
• remove ..._and_save() from FermiSolver methods
• defect_levels argument when annealing becomes a boolean
• docstrings
• multiprocessings
• tests
• suppress noisy warnings
• tutorials
• grid-searching
• effective dopant response
• "exceptions" for FermiSolverPyScFermi

[kavanase]

Looking good @alexsquires. I like the mix-in/abstract class approach.

The _interpolate_chempots() function is quite nice, and I think will likely be one of the mostly-used features from these additions. From what we were discussing before in the #doped Slack, would it be possible to also integrate a grid-scanning approach too? As in, the current function linearly interpolates between two points (chem pot limits), which is nice and v useful in many cases, but if for instance we have a 3D chemical potential space, our min/max of the property of interest could also lie somewhere in the middle of this space, rather than along any linear path between two vertices/limits.

Ofc the user could iterate over all possible vertex/limit combinations if they knew what they were doing, which would be a good stab at this and get you most of the way there I imagine (kind of like a 'band structure path' approach), but still is only covering certain 'high-symmetry' paths in chem pot space. So if a grid approach was also possible (where e.g. the user can just set the chempot spacing in eV, or total number of grid points), that would also be v nice I think – should be doable with some of the scipy grid space interpolation functions I think? And/or pymatgen chemical potential diagram methods maybe?
If it was possible to implement relatively painlessly, could we add these two options? ☝️ 🙏

Sorry to be piling on the requests, but I guess the other main use case I'd imagine for this part of the code would be for the more complex defect thermodynamics analysis that py-sc-fermi allows, where you want to fix certain defect/species etc concentrations and perform some constrained solutions. Is it possible to integrate this to the fermi_solver code?

Also just fyi, about your earlier _format_defect_name() question, yes it is actually quite externally useful and I'll make it a public function in the next (minor) release.

[alexsquires]

Yeah, dw this is all on the list, should be ready to go by the end of the week, I sort of trashed everything to start again (for about the 6th time now), and just adding everything back in piecewise to make sure it's all consistent with the latest thermodynamics object.

…

On Thu, 15 Feb 2024 at 04:58, Seán Kavanagh ***@***.***> wrote: Looking good @alexsquires <https://github.com/alexsquires>. I like the mix-in/abstract class approach. The _interpolate_chempots() function is quite nice, and I think will likely be one of the mostly-used features from these additions. From what we were discussing before in the #doped Slack, would it be possible to also integrate a grid-scanning approach too? As in, the current function linearly interpolates between two points (chem pot limits), which is nice and v useful in many cases, but if for instance we have a 3D chemical potential space, our min/max of the property of interest could also lie somewhere in the middle of this space, rather than along any linear path between two vertices/limits. Ofc the user could iterate over all possible vertex/limit combinations if they knew what they were doing, which would be a good stab at this and get you most of the way there I imagine (kind of like a 'band structure path' approach), but still is only covering certain 'high-symmetry' paths in chem pot space. So if a grid approach was also possible (where e.g. the user can just set the chempot spacing in eV, or total number of grid points), that would also be v nice I think – should be doable with some of the scipy grid space interpolation functions I think? And/or pymatgen chemical potential diagram methods maybe? If it was possible to implement relatively painlessly, could we add these two options? ☝️ 🙏 Sorry to be piling on the requests, but I guess the other main use case I'd imagine for this part of the code would be for the more complex defect thermodynamics analysis that py-sc-fermi allows, where you want to fix certain defect/species etc concentrations and perform some constrained solutions. Is it possible to integrate this to the fermi_solver code? Also just fyi, about your earlier _format_defect_name() question, yes it is actually quite externally useful and I'll make it a public function in the next (minor) release. — Reply to this email directly, view it on GitHub <#46 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFMK74XGZEUFF3TPF7MZHH3YTXZ5JAVCNFSM6AAAAABBVDPFSSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBVHE2TIMRVHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[alexsquires]

@adair-nicolson , could you re-parse your Cu2SiSe3 data now that everything is relatively final and I can add the grid searching stuff back in?

[alexsquires]

@kavanase proposing this as the full functionality - do you want to take a look at the example ipynb while I finish off the tests before I get to comfortable?

[kavanase]

@alexsquires going through now. One question, would it make sense to allow initialisation of FermiSolverDoped/PyScFermi objects using FermiSolver(..., backend="doped/py-sc-fermi") to make it a bit cleaner for the user?
Maybe defaulting to py-sc-fermi if installed, otherwise doped. If you agree, I can implement as it should just be a small update

[alexsquires]

Such a good idea that I'm embarrassed I didn't think of it before

[kavanase]

Ok cool will implement! Also to check, is there any point keeping multiplicity_scaling as an optional input? Like the default handling (with a minor update to allow some negligible numerical differences) should basically always be the desired usage?

        if multiplicity_scaling is None:
            ms = self.defect_thermodynamics.defect_entries[0].defect.structure.volume / self.volume
            # check multiplicity scaling is almost an integer
            if not np.isclose(ms, round(ms)):
                raise ValueError(
                    "The multiplicity scaling factor is not almost an integer. "
                    "Please specify a multiplicity scaling factor."
                )
            self.multiplicity_scaling = round(ms)
        else:
            self.multiplicity_scaling = multiplicity_scaling

[kavanase]

Also did you check with the latest code that the tutorial all works fine?

[alexsquires]

Ok cool will implement! Also to check, is there any point keeping multiplicity_scaling as an optional input? Like the default handling (with a minor update to allow some negligible numerical differences) should basically always be the desired usage?

        if multiplicity_scaling is None:
            ms = self.defect_thermodynamics.defect_entries[0].defect.structure.volume / self.volume
            # check multiplicity scaling is almost an integer
            if not np.isclose(ms, round(ms)):
                raise ValueError(
                    "The multiplicity scaling factor is not almost an integer. "
                    "Please specify a multiplicity scaling factor."
                )
            self.multiplicity_scaling = round(ms)
        else:
            self.multiplicity_scaling = multiplicity_scaling

Yeah, we can get rid of it. I suppose it is a hang over from my very "what if?" approach to py-sc-fermi where most of the fun was just playing around with the defect system and seeing how everthing changed, but if people want that, they can go to py-sc-fermi 🤓

On the thing of having fermisolver with a back-end option, I think the default should just be doped in all cases. This is doped after all, and it is marginally faster for the thing I reckon 90% of users will want, which is the convenience functions with quench+anneal.

Everything was working locally when I pushed, but I was in a bit of a frenzy, have you found something that doesn't work?

[kavanase]

Ok! And yeah I guess if it's an advanced enough user that they were playing around with things like that, they could also achieve the same effects other ways (e.g. by just adding an entry to DefectEntry.degeneracy_factors etc).

For using the doped backend as default, ok makes sense! (Especially if it's currently a bit quicker). I think it should be fairly easy to add a convenience method that converts from the doped backend to the py-sc-fermi one, so might try add this so that the doped backend can be used by default, and then switches to the py-sc-fermi one if needed (e.g. for free_defects) if it's installed. I guess the free_defects behaviour could also be added to the doped Fermi solving functions too, which in theory should be fairly straightforward.

For the last bit about something that doesn't work, it was just that I moved this _handle_chempots() line a couple lines up (because the chempots output was un-used so I assumed it was meant to be before when chempots is used), but this broke some things, so the line is just removed now and all seems to be working as expected.

doped/doped/interface/fermi_solver.py

Line 1451 in b8cf015

chempots = self._handle_chempots(chempots)

[kavanase]

Added some of the changes mentioned above now.

I played around with joblib/multiprocessing for this, with some profiling (though not so easy). It seems that thread locking and synchronization is causing the bottlenecks and slow speeds once attempting parallelization, which I think implies the need for shared resources but this shouldn't be the case afaik... Anyway it's pretty fast now as is. If someone needs to run it faster they can ask about multiprocessing or try themselves

Question: For scan_chempots, it's meant to be a list of chemical potentials that are then scanned over yeah? I guess we expect this to be a rare use case, with interpolate_chempots or scan_chemical_potential_grid usually preferred by the users. Either way, the current code is broken as it doesn't loop over the input list. Just checking there's still a use case for this function before I go trying to fix?

A benefit of the merge to FermiSolver with backend choice now is that we also minimise docstring duplication for the methods, so easier to maintain and update

[kavanase]

Ok some notes on the above. I updated the chempots/limit/el_refs inputs to be flexible and adopt a similar format to that in thermodynamics, to make things easier for the user, for maintenance etc.

I also (mostly, at least) fixed the scan_chempots() function mentioned above.

From playing around, I realised that ChemicalPotentialGrid and associated functions seems to only work for ternary compounds? e.g. with CdTe:

and Na2FePO4F:


(I've now added the get_doped_chempots_from_entries convenience function to chemical_potentials so can generate example chempots from the Materials Project quickly and easily this way, as shown).

Would you be able to look at this, so that it works for all dimensions?

Also for the tests, it would be most efficient to use a decorator (like this for testing both the new and legacy MP APIs in the chemical potentials tests) which tests each analysis function for both the doped and py-sc-fermi backends, rather than duplicating code. Would you mind adding this, and some tests for the scan_... functions (ideally with a variety of binary/ternary/quaternary+ systems)? 🙏

[kavanase]

@alexsquires something I was also thinking about. Do you think it still makes sense to have this under the separate interface module? Of course py-sc-fermi is one of the optional backends here, but most of the code is backend-agnostic. What about moving it into doped.thermodynamics?
I don't have any strong feelings about this, just thinking if it's the better category fit

[kavanase]

@alexsquires, I was looking again at the current code in this PR (hopefully nearly ready to go 🤞) as I'm currently trying to use it in some of my work (on selenium defects). I was hoping to use the py-sc-fermi use case where one can set a fixed concentration of a defect/impurity species, and solving with this constraint, but I think this is not implemented as far as I can tell?

I checked back and this was mentioned way back at the start of this PR, and the effective_dopant_concentration option was then added, but I think this other functionality wasn't?

Sorry to be piling on the requests, but I guess the other main use case I'd imagine for this part of the code would be for the more complex defect thermodynamics analysis that py-sc-fermi allows, where you want to fix certain defect/species etc concentrations and perform some constrained solutions. Is it possible to integrate this to the fermi_solver code?

Just checking I'm not missing something / avoiding redundant work. I can add this to the code if it's not there.

[alexsquires]

@alexsquires, I was looking again at the current code in this PR (hopefully nearly ready to go 🤞) as I'm currently trying to use it in some of my work (on selenium defects). I was hoping to use the py-sc-fermi use case where one can set a fixed concentration of a defect/impurity species, and solving with this constraint, but I think this is not implemented as far as I can tell?

I checked back and this was mentioned way back at the start of this PR, and the effective_dopant_concentration option was then added, but I think this other functionality wasn't?

Sorry to be piling on the requests, but I guess the other main use case I'd imagine for this part of the code would be for the more complex defect thermodynamics analysis that py-sc-fermi allows, where you want to fix certain defect/species etc concentrations and perform some constrained solutions. Is it possible to integrate this to the fermi_solver code?

Just checking I'm not missing something / avoiding redundant work. I can add this to the code if it's not there.

I think it would be nice to maybe have it accept a dictionary of defects and concentraitons, and optionally a charge state so e.g.

fixed_concentrations = {"v_Cd": x, "v_Te_+1" : y}

This could fairly easily be an additional kwarg of any of the py-sc-fermi methods?

@alexsquires something I was also thinking about. Do you think it still makes sense to have this under the separate interface module? Of course py-sc-fermi is one of the optional backends here, but most of the code is backend-agnostic. What about moving it into doped.thermodynamics? I don't have any strong feelings about this, just thinking if it's the better category fit

I also thinks this makes sense. I will fix everything up at the beginning of next week, then we could consider moving it?

[kavanase]

Sounds good!