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

Project Shape2SAS #3204

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft

Project Shape2SAS #3204

wants to merge 23 commits into from

Conversation

Qerneas
Copy link

@Qerneas Qerneas commented Feb 7, 2025

Description

Please include a summary of the change and which issue is fixed. List any dependencies that are required for this change.

This is a draft pull request for the Shape2SAS GUI. With Shape2SAS experimental SAXS data can be simulated and custom plugin models be generated by user by combining different subunits to create a model.

Fixes # (issue/issues)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)

Licencing (untick if necessary)

  • The introduced changes comply with SasView license (BSD 3-Clause)

First commit to SasView.

- GUI window (build model) to build a
  particle using Shape2SAS

- GUI window (Calculations) holding simulation aspects of simulated SAXS using Shape2SAS

- GUI window (Virtual SAXS Experiment) holding parameters for the simulated system.
Plugin model can now take parameters from the table and:

- set chosen in parameters to be fitted (if the user wants to) with a description for each variable.

- set names associated to fit variable as input to Iq function and set chosen variables and (chosen) constants in Shape2SAS.
Added the possibility to input a list containing q values by expanding the class Qsampling.

Like choosing the type of structure factor input, you know have to specify the q sampling method (uniform sampling or user sampled input) that is being used.
Updated genPlugin with regards to the class Qsampling
Updated spacer in ViewerModel.py to Expanding policy
Combined Calculation tab with Virtual SAXS Experiment tab

added Shape2SAS path in plugin model
added QstackedWidget to set options for Structure factor values.
Updated genPlugin.py names
Built a simple Constraint window.

The Constraint window will contain:
- A table to choose what parameter are to be fitted
- A textEdit window to write Translation to the plugin model.
- A textEdit window for error output
Rewrote enum class and included ModelEditor to Constraint Widget.
Conditions (MethodType), *args and **kwargs are introduced to the methods reading data from the subunit table.
The idea has been to make the these methods more generic. The methods can now handle both the plugin case and the simulating SAXS case by inputting a condition method to tell how the data should be handled.
SasView parameter Translation can now be written to the generated plugin model. The program will check requirements for including translation through different methods defined in Constraint.py
Added plot to virtual SAXS experiment and restricted the inputs in  virtual SAXS experiment for the different QlineEdits
Added missing tooltips to the button options class and pushbuttons added later to the class.
Reset button now resets the tab or window it was clicked from. Close closes all windows in Shape2SAS GUI
The program will nnow check if any subunits have been added, or if any fit parameters has been checked.
If parameters are checked but no constrain set, default constrains will be set for the fit parameters
Updated Translation so that constraints are directly inputted to the fit function
created Shape2SAS Constraint log to report on the state of the plugin model when being created.
added atomic form factor when simulating data and deleted files for testing purposes
Fetched with main to be up to date
Copy link
Contributor

@krzywon krzywon left a comment

Choose a reason for hiding this comment

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

I've looked through about half the code so far. I'll take a further look next week.

Attempting to generate the python files based on the UI, I am getting the following error. I am also unable to open ButtonOptions.ui and DesignWindow.ui in Qt Creator, both with similar errors.

uic: Error in line 15, column 57 : Unexpected element layout
File 'sasview\src\sas\qtgui\Perspectives\Shape2SAS\UI\ButtonOptionsUI.ui' is not valid
'Lib\site-packages\PySide6\uic -g python sasview\src\sas\qtgui\Perspectives\Shape2SAS\UI\ButtonOptionsUI.ui -o sasview\src\sas\qtgui\Perspectives\Shape2SAS\UI\ButtonOptionsUI.py' returned 1
Traceback (most recent call last):
File "sasview\run.py", line 98, in
rebuild_new_ui()
File "sasview\src\sas\qtgui\convertUI.py", line 69, in rebuild_new_ui
pyuic(file_in, file_out)
File "sasview\src\sas\qtgui\convertUI.py", line 39, in pyuic
subprocess.run(run_line, check=True)
File "Lib\subprocess.py", line 571, in run
raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['pyside6-uic', 'sasview\src\sas\qtgui\Perspectives\Shape2SAS\UI\ButtonOptionsUI.ui', '-o', 'sasview\src\sas\qtgui\Perspectives\Shape2SAS\UI\ButtonOptionsUI.py']' returned non-zero exit status 1


try:
ast.parse(text)
return True
Copy link
Contributor

Choose a reason for hiding this comment

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

This returns a boolean, but that boolean is never used in your calls. Remove? Otherwise, a more semantic method name should be isPythonSyntaxValid.

return importStatement, parameters, translation, checkedPars

@staticmethod
def getPosition(item: Union[str, float, int], itemLists: list[list[Union[str, float, int]]]) -> tuple[int, int]:
Copy link
Contributor

Choose a reason for hiding this comment

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

You use Union[str, float, int] a lot. You can create custom types to prevent this type of repetition.

VAL_TYPE = Union[str, float, in]
item: VAL_TYPE, ...

Comment on lines +131 to +140
removedPars = []
for item in listItems:
for statement in listToCompare:
if item in statement:
#NOTE: library names, "from" and "import" will also be removeds
removedPars.append(item)

for remove in removedPars:
if remove in listItems:
listItems.remove(remove)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minimizing loops (also, the list is never returned, but you're acting on a direct reference to the passed list, which is changing that list). One other thing I don't follow here - both listItems and listToCompare are lists of scalars, based on the type hints. Are you checking if the string is in another string here? If you are checking against the list items, the nested for loop shouldn't be needed.

Suggested change
removedPars = []
for item in listItems:
for statement in listToCompare:
if item in statement:
#NOTE: library names, "from" and "import" will also be removeds
removedPars.append(item)
for remove in removedPars:
if remove in listItems:
listItems.remove(remove)
finalPars = []
for item in listItems:
for statement in listToCompare:
if item not in statement:
finalPars.append(item)
return finalPars (or listItems = finalPars)

lines = [line.replace('\t', '').strip() for line in lines if line.strip()]

#Check parameters and update checkedPars
for i in range(len(lines)):
Copy link
Contributor

Choose a reason for hiding this comment

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

You only use i for indexing the lines list which is an operation against the list, which has a non-zero time and processor cost. Use for line in lines: to get the line a single time and compare that line directly.

Comment on lines +325 to +335
def logException(self, exception):
htmlise = "<br>".join(exception.split("\n"))
self.textEdit_2.append(f'<span style="color:Red;"><b>{htmlise}</b></span>')

def logWarning(self, message):
htmlise = "<br>".join(message.split("\n"))
self.textEdit_2.append(f'<span style="color:Orange;"><b>Warning: {htmlise}</b></span>')

def logInfo(self, message):
htmlise = "<br>".join(message.split("\n"))
self.textEdit_2.append(f'<span style="color:Black;"><b>Info: </b>{htmlise}</span>')
Copy link
Contributor

Choose a reason for hiding this comment

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

To tie into the SasView log, simply use the logging package. Extra logs will only cause more confusion.

logger = logging.getLogger(__name__)

...

    logger.<info|warn|error>(message)

Fixed getParameters method in Constraint.py by using ast insteand of re to get parameters due to re being time consuming.

added cylinder ring as a subunit
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