View Issue Details

IDProjectCategoryView StatusLast Update
0000423ascendpygtk guipublic2012-04-28 10:25
Reporterjohn 
Assigned To 
PrioritynormalSeveritytweakReproducibilityalways
Status confirmedResolutionopen 
Product Version0.9.7 
Target Version1.0Fixed in Version 
Summary0000423: Changing value of solved variable should force removal of 'solved' indication
DescriptionWhen using the 'properties' dialog box to edit the value of a variable that has already been solved, the variable should afterwards be marked as 'not solved'. Currently, in the PyGTK GUI, the 'tick' or 'check' mark next to the variable remains there, even though the variable is no longer correctly solved.
Additional InformationShould be a simple case of updating the relevant solver/var.h flags via C++ from the Python code.
TagsNo tags attached.

Relationships

related to 0000507 new propagate var/relation 'solved' status up through model hierarchy in processVarStatus 

Activities

john

2011-03-09 19:33

administrator   ~0000688

Ideally, the block structure from the last solution could be used to un-flag all 'downstream' vars as well... Have to be careful about how that works with non-QRSlv solvers though.

john

2011-04-04 09:30

administrator   ~0000704

I believe that the problem here is arising (a) when the "auto" button is not pushed, and (b) when the "properties" dialog is used to set the value, rather than direct input.

aakash

2011-04-06 20:41

developer  

fix423.diff (2,457 bytes)
Index: ascxx/simulation.cpp
===================================================================
--- ascxx/simulation.cpp	(revision 3357)
+++ ascxx/simulation.cpp	(working copy)
@@ -1000,7 +1000,59 @@
 				
 	//CONSOLE_DEBUG(" ...done var status");
 }
+void
+Simulation::processVarStatusUnsolved(){
+	if(!sys)return;
 
+	//CONSOLE_DEBUG("setting var status'");
+
+	var_variable **vlist = slv_get_solvers_var_list(getSystem());
+	int nvars = slv_get_num_solvers_vars(getSystem());
+
+	rel_relation **rlist = slv_get_solvers_rel_list(getSystem());
+	int nrels = slv_get_num_solvers_rels(getSystem());
+
+	slv_status_t status;
+	if(slv_get_status(sys, &status)){
+		ERROR_REPORTER_HERE(ASC_PROG_ERR,"Unable to update var status (get_status returns error)");
+		return;
+	}
+
+	if(status.block.number_of == 0){
+		cerr << "Variable statuses can't be set: block structure not yet determined." << endl;
+		return;
+	}else{
+#if SIMULATION_DEBUG
+		CONSOLE_DEBUG("There are %d blocks", status.block.number_of);
+#endif
+	}
+
+	
+	for(int c=0; c < nvars; ++c){
+		var_variable *v = vlist[c];
+		Instanc i((Instance *)var_instance(v));
+		InstanceStatus s = ASCXX_INST_STATUS_UNKNOWN;
+		if(i.isFixed())
+			s = ASCXX_VAR_FIXED;
+		else
+			s = ASCXX_VAR_UNSOLVED;
+		i.setStatus(s);
+	}
+
+	for(int j=0; j < nrels; ++j){
+		rel_relation *r = rlist[j];
+		Instanc i((Instance *)rel_instance(r));
+		InstanceStatus s = ASCXX_INST_STATUS_UNKNOWN;
+		if(rel_in_when(r)){
+			if(!rel_active(r)){
+				s = ASCXX_REL_INACTIVE;
+			}				
+		}
+		i.setStatus(s);
+	}
+				
+	//CONSOLE_DEBUG(" ...done var status");
+}
 void
 Simulation::setSolverHooks(SolverHooks *H){
 #if SIMULATION_DEBUG
Index: ascxx/simulation.h
===================================================================
--- ascxx/simulation.h	(revision 3357)
+++ ascxx/simulation.h	(working copy)
@@ -116,6 +116,7 @@
 	const std::string getInstanceName(const Instanc &) const;
 
 	void processVarStatus();
+	void processVarStatusUnsolved();
 	const int getNumVars();
 
 	const int getActiveBlock() const;
Index: pygtk/gtkbrowser.py
===================================================================
--- pygtk/gtkbrowser.py	(revision 3357)
+++ pygtk/gtkbrowser.py	(working copy)
@@ -688,7 +688,7 @@
 			self.do_solve()
 		else:
 			try:
-				self.sim.processVarStatus()
+				self.sim.processVarStatusUnsolved()
 			except RuntimeError,e:
 				self.reporter.reportError(str(e))
 			self.modelview.refreshtree()
fix423.diff (2,457 bytes)

aakash

2011-04-06 20:45

developer   ~0000715

This bug can be fixed by the .diff I have attached.

I have defined a new function in simulation.cpp. It is called every time a change in value or type of any variable occurs. (The new function may have some unnecessary lines of code, please review it)

I have tested this using solver engine QRSlv on Ubuntu 10.04.

john

2011-04-12 00:09

administrator   ~0000727

Although I imagine the patch probably works, I was looking for a more fundamental solution. The 'processVarStatus' command is used to update 'interface pointer' data containing the status of a given variable using information from the solver.

If we move to a bitfield based approach for the 'setStatus' on variables, we could then store a bit corresponding to variables modified but not yet solved. When the GUI was used to change a value, it could thus be marked 'tainted'.

The processVarStatus could then use that extra information in a sensible way to mark all variables in the tainted block and any subsequent blocks as unsolved.

Ideally, the 'tainting' could be passed all the way back to the solver, so that the solver could avoid unneeded work. But that's probably more trouble than it's worth.

aakash

2011-04-12 00:16

developer   ~0000728

Last edited: 2011-04-12 00:22

View 2 revisions

That could be done, but as you said, it's probably not worth it. And, it would also make it dependent on the solver. So, maybe we can use the patch as a more practical solution to it.

john

2011-04-12 00:41

administrator   ~0000729

One problem with the patch (I need to check it) is that a call to 'processVarStatus' overwrites the result of 'processVarStatusUnsolved', so possibly other actions could reverse the flagging from your function.

aakash

2011-04-12 00:46

developer   ~0000730

Last edited: 2011-04-12 00:54

View 3 revisions

As far as I have seen, processVarStatus is only called while solving, running a method and intregating. In those cases, the flags need to be reset anyways, so it should not matter.

There was a call to it when auto solve was disabled, I have changed that call in the patch.

ben

2012-04-05 03:15

manager   ~0000844

Email from: Ben Allan <ba22.cmu@gmail.com>

The whole concept of marking a *variable* as solved is inappropriate
in almost all modeling contexts, in part for the problems illustrated
in the bug report. Best solution is get rid of the flag and certainly
to get rid of showing it to the user.

In unpartitioned systems, all equations (and variables) are solved or
none of them are. In partitioned systems, some upstream equation
blocks may be 'solved' to a tolerance that makes downstream blocks
infeasible even though the whole system could converge if solved
unpartitioned or solved with tighter tolerances upstream.

ben


On Tue, Mar 27, 2012 at 1:35 PM, Mantis Bug Tracker <bugs@ascend4.org> wrote:
>
> Issue 0000423 is now monitored by user saheb.
> [EmailReporting -> Removed part identified as reply]

john

2012-04-05 04:01

administrator   ~0000846

I think that reporting of solved/unsolved/unvisited variables to the user is extremely useful in the user interface for debugging purposes. This is about reporting what the solver managed to achieve and where it got stuck. In my experience, 9 times out of 10, this ends up telling you that you have an equation that's written incorrectly, or some bounds that don't make sense, or something non-physical that's happening, etc.

I think it's really important and useful to report this. Then, when playing around with the model, if values of fixed variables have been changed, it should be fairly easy to re-query the solver to ask what downstream variables are affected by that changed value, so that the whole model shouldn't have to be resolved.

This is not about any changes to the solver or compiler API, it's just about using the solver return status in a smart way in the GUI.

Incidentally there are cool things that we can do here in relation to DAE systems too... we should be partitioning the part of a DAE model that can be solved with QRSlv (the non-dynamic part of the model, BEFORE the DAE solution, then the post-processing part of the model, AFTER the DAE solution). That could also be usefully indicated to the user via icons next to variables in the instance tree.

Issue History

Date Modified Username Field Change
2010-02-23 09:53 john New Issue
2010-02-23 09:53 john Status new => assigned
2010-02-23 09:53 john Assigned To => john
2010-02-24 18:14 john Assigned To john =>
2010-02-24 18:14 john Status assigned => new
2010-03-10 10:46 john Status new => confirmed
2010-03-23 18:24 john Target Version => 0.9.8
2011-03-09 19:33 john Note Added: 0000688
2011-04-04 09:30 john Note Added: 0000704
2011-04-06 20:41 aakash File Added: fix423.diff
2011-04-06 20:45 aakash Note Added: 0000715
2011-04-12 00:09 john Note Added: 0000727
2011-04-12 00:16 aakash Note Added: 0000728
2011-04-12 00:22 aakash Note Edited: 0000728 View Revisions
2011-04-12 00:39 john Relationship added related to 0000507
2011-04-12 00:41 john Note Added: 0000729
2011-04-12 00:46 aakash Note Added: 0000730
2011-04-12 00:51 aakash Note Edited: 0000730 View Revisions
2011-04-12 00:54 aakash Note Edited: 0000730 View Revisions
2012-04-05 03:15 ben Note Added: 0000844
2012-04-05 04:01 john Note Added: 0000846
2012-04-28 10:25 john Target Version 0.9.8 => 1.0