Skip to content
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

Adding components to the Solution #105

Open
GarzonDiegoFEUP opened this issue Sep 27, 2024 · 2 comments
Open

Adding components to the Solution #105

GarzonDiegoFEUP opened this issue Sep 27, 2024 · 2 comments
Assignees

Comments

@GarzonDiegoFEUP
Copy link
Collaborator

GarzonDiegoFEUP commented Sep 27, 2024

I have being exploring the Solution class, and I found these possible issues:

  1. When doing an EntryPoint for a solution, if you try to modify the solutes, an error will appear and the entry will not work anymore. I know the proper way will be to add a component, but the solvent list should not be editable.
  2. The solute and the solvent should be add at the same time (without saving in between), because then the molar concentration cannot be calculated. So maybe there should be a safe check to only calculate the molar concentration when both solvent and solute are not empty.
  3. In the ThinFilmStack class you add Substrate and layers, instead of components so it works the other way around as solutions, which I don't think should be the case.

Thank you @aalbino2 @ka-sarthak !!

@ka-sarthak
Copy link
Collaborator

ka-sarthak commented Sep 30, 2024

Hi @GarzonDiegoFEUP, your feedback on the solution classes is much appreciated.

  1. You are right that solute and solvent should be non-editable to avoid confusion. We don't have an annotation to make sub-sections non-editable. But here's an issue you can follow regarding it: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/issues/2151
  2. The molar concentration should be calculated if the volume is available. If you add just a solute and save it, the molar concentration should not work because the volume is missing. In fact as I test it now, I get a warning saying "Volume of the solution is missing, can not calculate the concentration of the component ClNa." as expected, but I also get an unexpected error from self.combine_components(self.solutes, logger). I will take a look into this. Add solutions category and resolve bug in combine_components #106

@ka-sarthak
Copy link
Collaborator

  1. The issue was in the MSection.m_copy() method in nomad-lab. Here's an MR to fix it: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/2127. Once we update the nomad version in the oasis, this should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants