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

Deleting all variables doesn't work if one variable is derived from another (in GUI). #897

Closed
cameronsmith1 opened this Issue Nov 14, 2014 · 21 comments

Comments

Projects
None yet
6 participants
@cameronsmith1

cameronsmith1 commented Nov 14, 2014

In the UVCDAT GUI, I had a few variables and wanted to delete them all. Hence, I clicked the select_all, and hit delete. However, it produced an error that some variables were derived from other variables (which was true because I had generated variables that were averages of the original variables). This raises two issues: (1) I expected that the new variable would be independent of the ones it is derived from, which makes me worry that I have to worry about variable dependencies, and (2) why should it matter if I am deleting all of the variables? Note: I am using version 2.0.0.

@williams13

This comment has been minimized.

Contributor

williams13 commented Nov 14, 2014

I confirmed that if you derived a variable, say a = clt + 100. Select all and delete all you get:

clt is used to derive other variables. Delete those first.

Rémi can you look at this?

@doutriaux1 doutriaux1 added this to the 2.2 milestone Nov 14, 2014

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 24, 2015

@vvpalav any progress on this?

@hgohil2805 hgohil2805 assigned hgohil2805 and unassigned vvpalav Feb 26, 2015

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 26, 2015

(1) The issue with this is that the new variable is built on the basis of the old one and the old one is kept in memory for provenance purpose. I believe the new variable creation along with some other new variable dependent functionalities were designed with the same approach in mind and changing it around now might affect other things and can lead to a lot of problems on other parts of the application.

(2)True, the check's should not happen when all the variables are selected. I have changed the code to meet this requirement. Unfortunately I don't have the latest build (I am building a fresh one now).I will upload the changes from the newer build as soon as it's done.

@cameronsmith1

This comment has been minimized.

cameronsmith1 commented Feb 26, 2015

I have since had the related experience of trying to delete a subset of variables that include such dependencies. Will the new code work in this case too?
I envision an iterative deletion process in which any of the selected variable that can be deleted is done so, and then it loops around again and again, until no more selected variables can be deleted.

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 26, 2015

This is how it works

If you select all the variables for deletion - no checks are performed and you can delete all of them without any pop up errors.
However, if you select a subset of variables (which have dependencies) then it will check for dependencies. It wont be right of me to skip these checks here as the user has to be notified of the reason for not deleting the variable. I wasn't able to complete the build right now, I will anyways create a branch so that you can test it

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 26, 2015

One work around is you can select the dependent variables first and then select the original variables because as your rightly said, this is an iterative process and the code checks for dependent variables one by one as they are on the list.

@cameronsmith1

This comment has been minimized.

cameronsmith1 commented Feb 26, 2015

Is it possible to automate the last process, ie catch the errors while going through the list, but don't abort? Then, go back through the list and see if any of the previous dependencies/errors have been resolved. This would also work when the user wants to delete-all.

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 26, 2015

Yes, that is something that I am doing now. It's not very efficient but I think it will work.

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 26, 2015

I've created a branch with the changes called

removing_variables_and_dependent_variables.

It's not updated and I am not sure if you would be able to use it. Lemme know if you can or cannot.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 26, 2015

@hgohil2805 when you issue a PR i can try to bring it for @cameronsmith1 to test.

@cameronsmith1

This comment has been minimized.

cameronsmith1 commented Feb 26, 2015

Thanks, All.

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 26, 2015

@doutriaux1 I tried to issue a PR but it shows a lot more changed files than it should (probably because I am using an old build). Do you want me to issue one anyways? (I am not that good with git)

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 27, 2015

@hgohil2805

cdoutrix@sierra6:[/usr/gapps/uvcdat/chaos_5_x86_64_ib/default/2015-02-26/vistrails]:[uvcdat-master]:[3740]> git merge --no-ff removing_variables_and_dependent_variables
Auto-merging .gitignore
CONFLICT (add/add): Merge conflict in .gitignore
Auto-merging vistrails/core/resources/default_vistrails_startup_xml
CONFLICT (add/add): Merge conflict in vistrails/core/resources/default_vistrails_startup_xml
Auto-merging vistrails/core/uvcdat/plotmanager.py
CONFLICT (add/add): Merge conflict in vistrails/core/uvcdat/plotmanager.py
Auto-merging vistrails/gui/collection/workspace.py
CONFLICT (add/add): Merge conflict in vistrails/gui/collection/workspace.py
Auto-merging vistrails/gui/mashups/mashup_app.py
CONFLICT (add/add): Merge conflict in vistrails/gui/mashups/mashup_app.py
Auto-merging vistrails/gui/publishing.py
CONFLICT (add/add): Merge conflict in vistrails/gui/publishing.py
Auto-merging vistrails/gui/uvcdat/colormapEditorWidget.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/colormapEditorWidget.py
Auto-merging vistrails/gui/uvcdat/definedVariableWidget.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/definedVariableWidget.py
Auto-merging vistrails/gui/uvcdat/dockplot.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/dockplot.py
Auto-merging vistrails/gui/uvcdat/editorGraphicsMethodsWidget.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/editorGraphicsMethodsWidget.py
Auto-merging vistrails/gui/uvcdat/graphicsMethodsWidgets.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/graphicsMethodsWidgets.py
Auto-merging vistrails/gui/uvcdat/mainwindow.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/mainwindow.py
Auto-merging vistrails/gui/uvcdat/plotViewWidget.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/plotViewWidget.py
Auto-merging vistrails/gui/uvcdat/project_controller.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/project_controller.py
Auto-merging vistrails/gui/uvcdat/uvcdatCommons.py
CONFLICT (add/add): Merge conflict in vistrails/gui/uvcdat/uvcdatCommons.py
Auto-merging vistrails/packages/CLTools/wizard.py
CONFLICT (add/add): Merge conflict in vistrails/packages/CLTools/wizard.py
Auto-merging vistrails/packages/cdat/init.py
CONFLICT (add/add): Merge conflict in vistrails/packages/cdat/init.py
Auto-merging vistrails/packages/cdat/scripts/xml/canvas.xml
CONFLICT (add/add): Merge conflict in vistrails/packages/cdat/scripts/xml/canvas.xml
Auto-merging vistrails/packages/spreadsheet/spreadsheet_cell.py
CONFLICT (add/add): Merge conflict in vistrails/packages/spreadsheet/spreadsheet_cell.py
Auto-merging vistrails/packages/spreadsheet/spreadsheet_tabcontroller.py
CONFLICT (add/add): Merge conflict in vistrails/packages/spreadsheet/spreadsheet_tabcontroller.py
Auto-merging vistrails/packages/uvcdat_cdms/init.py
CONFLICT (add/add): Merge conflict in vistrails/packages/uvcdat_cdms/init.py
Auto-merging vistrails/packages/uvcdat_cdms/pipeline_helper.py
CONFLICT (add/add): Merge conflict in vistrails/packages/uvcdat_cdms/pipeline_helper.py
Auto-merging vistrails/packages/uvcdat_cdms/widgets.py
CONFLICT (add/add): Merge conflict in vistrails/packages/uvcdat_cdms/widgets.py
Auto-merging vistrails/packages/vtDV3D/CDMS_DatasetReaders.py
CONFLICT (add/add): Merge conflict in vistrails/packages/vtDV3D/CDMS_DatasetReaders.py
Auto-merging vistrails/packages/vtDV3D/InteractiveConfiguration.py
CONFLICT (add/add): Merge conflict in vistrails/packages/vtDV3D/InteractiveConfiguration.py
Auto-merging vistrails/packages/vtDV3D/__init__.py
CONFLICT (add/add): Merge conflict in vistrails/packages/vtDV3D/__init__.py
Auto-merging vistrails/packages/vtk/init.py
CONFLICT (add/add): Merge conflict in vistrails/packages/vtk/init.py
Auto-merging vistrails/packages/vtk/vtk_parser.py
CONFLICT (add/add): Merge conflict in vistrails/packages/vtk/vtk_parser.py
Auto-merging vistrails/packages/vtk/vtkcell.py
CONFLICT (add/add): Merge conflict in vistrails/packages/vtk/vtkcell.py
Automatic merge failed; fix conflicts and then commit the result.
@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 27, 2015

@hgohil2805 I created removing_variables_and_dependent_variables_rebased please make sure I didn't mess you up

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 27, 2015

@doutriaux1 Thanks a lot!. I was able to create the pull request. @cameronsmith1 hopefully you will be able to test it now:)

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 27, 2015

@hgohil2805 @cameronsmith1 will test soon it's on a computer he can access now. We should know tomorrow or by monday

@cameronsmith1

This comment has been minimized.

cameronsmith1 commented Feb 27, 2015

Partial success. If I load a variable 'T', and then create dependent variables t1 = 2 * T and t2 = 2 * t1, then the deletion of t1 and t2 works as intended. However, if I make the dependencies 3 deep, eg t3 = 2 * t2, and try to delete t1, t2, and t3, then it only deletes t2 and t3 and complains about t1. It would be nice to be infinitely recursive, but looping several times (4? 5? 6?) is probably good enough in practice.

Separately, I found that if I just tried creating new variables like ta = T, then UVCDAT got very confused, and wouldn't let me delete anything. I don't know whether this is a bug in the new code, or a consequence of some pre-existing bug.

Separately again, I get a seg-fault whenever I try to plot something. This is the first time I have used UVCDAT on this system, so I don't know whether this is related or not.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 27, 2015

I could plot with uvcdat-master, I will revert to that and you can try plotting again. If it still fails I suspect something wrong with your x forwarding. Otherwise it might be the branch.

@hgohil2805

This comment has been minimized.

hgohil2805 commented Feb 27, 2015

@cameronsmith1
1 - Yup, that should be how it should be. Will change it soon.

2 - Well that should not really happen. I will try and create new variables in a similar manner and test it.
3 - Probably what @doutriaux1 said. I don't think it is related to these variables. I'll try to double check

Also, Thanks for testing it quick:)

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Mar 9, 2015

@cameronsmith1 it should be up and running on rhea with the fix from @hgohil2805 at:
/lustre/atlas/world-shared/csc121/uvcdat/2015-03-05
when you get a chance can you please let us know if that's better for you.

@cameronsmith1

This comment has been minimized.

cameronsmith1 commented Mar 13, 2015

Yes, the deletions now seems to be working as expected, except for when I set a = b, and then it gets confused (eg, it will let me delete b but not a).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment