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

creating function main #4

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions diffpy/snmf/stretchednmfapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ def create_parser():
help="The directory where the results will be written. Defaults to '<input_directory>/snmf_results'.")
parser.add_argument('-t', '--data-type', type=str, choices=['powder_diffraction', 'pdf'],
help="The type of the experimental data.")
parser.add_argument('-l', '--lift', type=float, default=1,
help="The factor that determines how much the data is lifted. By default, the data will be vertically translated to make the minimum value 0.")
Copy link
Contributor

Choose a reason for hiding this comment

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

how about "The lifting factor. Data will be lifted by lifted data = data - min(data)*lift. Default is 1"

btw, it occurs to me that if the min(data) is positive, the "lift" will "lower" the data to zero if lift = 1 and will lower it to below zero if lift > 1. This is probably undesirable behavior. Make sure you have tests for this eventuality that give the behavior you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will make this changes.

The same thought occurred to me. If the min(data) is positive, it's not apparent to me that the data would need to be lifted. My idea is that if the minimum is positive, then do nothing to the data.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is reasonable, though why not allow the user to lift the data if she wants to?

Maybe make sure the lift is a lift, so it adds the absolute value of min(data)*lift to the data, something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

parser.add_argument('components', type=int,
sbillinge marked this conversation as resolved.
Show resolved Hide resolved
help="The number of component signals for the NMF decomposition. Must be an integer greater than 0.")
parser.add_argument('-v', '--version', action='version', help='Print the software version number')
Expand All @@ -36,17 +38,17 @@ def main():
variables = initialize_variables(data_input, args.components, data_type)

if variables["data_type"] == 'pdf':
sbillinge marked this conversation as resolved.
Show resolved Hide resolved
lifted_data = data_input - np.ndarray.min(data_input[:]) # Will later use to function in subroutines
lifted_data = data_input - np.ndarray.min(data_input[:]) # Will later use the lift_data function in subroutines

maxiter = 300

for out_iter in range(maxiter):
variables["weights_matrix"] = update_weights_matrix(variables["number_of_components"],
variables["signal_length"],
variables["stretching_factor_matrix"],
variables["component_matrix"], lifted_data,
variables["number_of_moments"], variables["weights_matrix"],
None)
variables["signal_length"],
variables["stretching_factor_matrix"],
variables["component_matrix"], lifted_data,
variables["number_of_moments"], variables["weights_matrix"],
None)

residual_matrix = get_residual_matrix(variables["component_matrix"], variables["weights_matrix"],
variables["stretching_factor_matrix"], lifted_data,
Expand Down