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

Naming convention, adding data from website, and CsI test #159

Merged
merged 13 commits into from
May 20, 2024

Conversation

aidanfitz
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@parikshitbajpai parikshitbajpai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Do we have permission to share all the data files that are new?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should rename it similar to other files and use elements instead of NobleMetals

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about AlMg-Liang.dat? If we do this for all the files we should get the data files sorted by elements and the author names will still be acknowledged.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CsI-Pham.dat?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZrH-Dupin.dat

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CuFeC-Kang.dat?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MoPdRuRhTc-Kaye.dat

README.md Outdated
Once the Ubuntu app is successfully installed, you can follow the [Building Thermochimica](#building-thermochimica) instructions below.
When Ubuntu app is successfully installed, run the following.
```bash
sudo apt-get update
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sudo apt-get update
sudo apt update && sudo apt upgrade

@@ -12,15 +12,15 @@
phaseIncludeTol = 1e-8

class CalculationWindow:
def __init__(self, parent, datafile, nElements, elements, active):
def __init__(self, parent, datafile, nElements, elements, active,scrollable=True):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def __init__(self, parent, datafile, nElements, elements, active,scrollable=True):
def __init__(self, parent, datafile, nElements, elements, active, scrollable=True):

self.parent = parent
self.datafile = datafile
self.nElements = nElements
self.elements = elements
self.active = active
if self.active:
self.makeLayout()
self.sgw = sg.Window(f'Phase Diagram Setup: {os.path.basename(self.datafile)}', self.layout, location = [400,0], finalize=True)
self.sgw = sg.Window(f'Phase Diagram Setup: {os.path.basename(self.datafile)}', self.layout,resizable=True,size=(600, 800),location = [400,0], finalize=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.sgw = sg.Window(f'Phase Diagram Setup: {os.path.basename(self.datafile)}', self.layout,resizable=True,size=(600, 800),location = [400,0], finalize=True)
self.sgw = sg.Window(f'Phase Diagram Setup: {os.path.basename(self.datafile)}', self.layout, resizable=True, size=(600,800), location = [400,0], finalize=True)

@@ -254,8 +254,8 @@ def makeLayout(self):
if (self.nElements < 8):
elemLayout = [sg.Column(elem1Layout),sg.Column(elem2Layout)]
else:
elemLayout = [sg.Column(elem1Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, expand_y = True),
sg.Column(elem2Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, expand_y = True)]
elemLayout = [sg.Column(elem1Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, size=(200, 100),expand_y = True),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
elemLayout = [sg.Column(elem1Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, size=(200, 100),expand_y = True),
elemLayout = [sg.Column(elem1Layout, vertical_alignment='t', scrollable = True, vertical_scroll_only = True, size=(200,100), expand_y = True),

elemLayout = [sg.Column(elem1Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, expand_y = True),
sg.Column(elem2Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, expand_y = True)]
elemLayout = [sg.Column(elem1Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, size=(200, 100),expand_y = True),
sg.Column(elem2Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, size=(200, 100), expand_y = True)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sg.Column(elem2Layout,vertical_alignment='t', scrollable = True, vertical_scroll_only = True, size=(200, 100), expand_y = True)]
sg.Column(elem2Layout, vertical_alignment='t', scrollable = True, vertical_scroll_only=True, size=(200,100), expand_y=True)]

@aidanfitz
Copy link
Collaborator Author

Sorry for the late response. I will get on it and make sure your suggestions will be used. I updated the file names to system-owner.dat. And all files are from the tutorial, excluding one that we cannot post there should be no need to worry on that end.

@parikshitbajpai parikshitbajpai merged commit c264155 into ORNL-CEES:master May 20, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants