Modifications to support logfile for MI output and other minor cleanup #1

rjplevin · 2016-07-28T14:20:21Z

I made the changes we discussed. In one case I tweaked the indentation so I could see what was going on. I realize that expands the surface area of the diff, so let me know how you have tabs set up in VIM so I can configure emacs similarly to avoid gratuitous changes in the future.

The jopt-simple cmdline parser is pretty nice. It might be useful in XMLDBDriver, too.

The patches you sent me also show up as changes here. I also fixed a couple of spelling errors since I noticed them. Let me know if you'd rather I didn't. :~)

pralitp

With regards to the .jar files:

When you built the .jar files which version of javac did you use / target? I've been targeting 1.6 to allow the old Apply JDK to continue to work.
Did anything really change with CSVToXML.jar (it will show up changed everytime it is rebuilt even if the underlying source files did not change). If not let's just revert it.

pralitp · 2016-09-23T19:29:10Z

cvs/objects/java/source/RunQueries.java

        // Run the ModelInterface in batch mode
-        String[] args = { "-b", mBatchFile };
-        InterfaceMain.main( args );
+        // String[] args = { "-b", mBatchFile };


We can go ahead and get rid of this commented out code

pralitp · 2016-09-23T19:34:58Z

input/gcam-data-system/_common/ModelInterface/src/MANIFEST_MI.MK

-Class-Path: jars/BaseX.jar jars/poi-contrib-3.0-alpha1.jar jars/jcommo
- n-1.0.0-rc1.jar jars/poi-scratchpad-3.0-alpha1.jar jars/jfreechart-1.
- 0.0-pre2.jar jars/xalan.jar jars/poi-3.0-alpha1.jar
+Class-Path: jars/BaseX.jar jars/poi-contrib-3.0-alpha1.jar jars/jcommon-1.0.0-rc1.jar jars/poi-scratchpad-3.0-alpha1.jar jars/jfreechart-1.0.0-pre2.jar jars/xalan.jar jars/poi-3.0-alpha1.jar jars/jopt-simple-5.0.2.jar


It has been my experience in the past that these manifest files are picky about the fixed width.

pralitp · 2016-09-23T19:36:19Z

input/gcam-data-system/_common/ModelInterface/src/ModelInterface/ModelGUI2/DbViewer.java

@@ -1763,6 +1763,7 @@ public void runBatch(Node command) {
 						// should I print this error to the screen?
 					}
 				}
+                boolean wasDBOpened = false;


Given these changes are being incorporated in another branch (with some other fixes as well) it would be best to just revert these changes here.

pralitp · 2016-09-23T19:37:41Z

input/gcam-data-system/_common/ModelInterface/src/ModelInterface/ModelGUI2/xmldb/XMLDB.java

@@ -146,6 +146,14 @@ private void openDB(String dbPath, Context contextIn) throws Exception {
        // and use it as the base path for finding all collections/containers
        System.setProperty("org.basex.DBPATH", path);

+        // TODO: worry about spaces?


I assume this is the same as the bug fix above? If so we should revert these as well so as to avoid merge conflicts later.

pralitp · 2016-09-23T19:42:12Z

input/gcam-data-system/_common/ModelInterface/src/ModelInterface/InterfaceMain.java

+		    main.menuAdders.add(inputView);
+
+		    PrintStream stdout = System.out;
+		    if (opts.has("l")) {


I guess I don't have a strong opinion on this but should we move this up? Otherwise messages prior would still show up on the console instead of log. As far as I can tell this would just be the "Running headless?" message but if there were more diagnostics in say the constructor for DbViewer then those would get misplaced.

pralitp · 2016-09-23T19:43:10Z

cvs/objects/java/source/RunQueries.java

        mBatchFile = aBatchFile;
+	mLogFile = aLogFile;


Check tabs vs spacing

rjplevin · 2016-09-23T20:10:20Z

I assume you'll get this via github, but just in case, I've cc'd you...

See below.

On Sep 23, 2016, at 12:47 PM, Pralit Patel notifications@github.com wrote:

@pralitp commented on this pull request.

With regards to the .jar files:

When you built the .jar files which version of javac did you use / target? I've been targeting 1.6 to allow the old Apply JDK to continue to work.
I used 1.8 because that's what I'm running on my Mac. I would be surprised if the code is sensitive to that, but who knows.

Did anything really change with CSVToXML.jar (it will show up changed everytime it is rebuilt even if the underlying source files did not change). If not let's just revert it.
Nope; didn't touch it.

In cvs/objects/java/source/RunQueries.java #1 (review):
     // Run the ModelInterface in batch mode
   String[] args = { "-b", mBatchFile };
   InterfaceMain.main( args );
   // String[] args = { "-b", mBatchFile };
We can go ahead and get rid of this commented out code
Yup.

In input/gcam-data-system/_common/ModelInterface/src/MANIFEST_MI.MK #1 (review):

@@ -1,8 +1,6 @@
Manifest-Version: 1.0
Sealed: true
-Class-Path: jars/BaseX.jar jars/poi-contrib-3.0-alpha1.jar jars/jcommo

n-1.0.0-rc1.jar jars/poi-scratchpad-3.0-alpha1.jar jars/jfreechart-1.

0.0-pre2.jar jars/xalan.jar jars/poi-3.0-alpha1.jar
+Class-Path: jars/BaseX.jar jars/poi-contrib-3.0-alpha1.jar jars/jcommon-1.0.0-rc1.jar jars/poi-scratchpad-3.0-alpha1.jar jars/jfreechart-1.0.0-pre2.jar jars/xalan.jar jars/poi-3.0-alpha1.jar jars/jopt-simple-5.0.2.jar
It has been my experience in the past that these manifest files are picky about the fixed width.

This is my only experience with it, so whatever works!

In input/gcam-data-system/_common/ModelInterface/src/ModelInterface/ModelGUI2/DbViewer.java #1 (review):
@@ -1763,6 +1763,7 @@ public void runBatch(Node command) {
// should I print this error to the screen?
}
}
           boolean wasDBOpened = false;
Given these changes are being incorporated in another branch (with some other fixes as well) it would be best to just revert these changes here.
In input/gcam-data-system/_common/ModelInterface/src/ModelInterface/ModelGUI2/xmldb/XMLDB.java #1 (review):

@@ -146,6 +146,14 @@ private void openDB(String dbPath, Context contextIn) throws Exception {
// and use it as the base path for finding all collections/containers
System.setProperty("org.basex.DBPATH", path);
   // TODO: worry about spaces?
I assume this is the same as the bug fix above? If so we should revert these as well so as to avoid merge conflicts later.
With both of these, I don't see changes that I would have made. Could this be due to some difference between the public github repo and your internal one?

In input/gcam-data-system/_common/ModelInterface/src/ModelInterface/InterfaceMain.java #1 (review):
      System.setProperty("java.awt.headless", "true");
    System.out.println("Running headless? "+GraphicsEnvironment.isHeadless());
    File batchFile = new File(filename);
    main  = new InterfaceMain();
    // Construct the subset of menu adders that are also BatchRunner while
    // avoiding creating any GUI components
    // TODO: avoid code duplication
    final MenuAdder dbView = new DbViewer();
    final MenuAdder inputView = new InputViewer();
    main.menuAdders = new ArrayList<MenuAdder>(2);
    main.menuAdders.add(dbView);
    main.menuAdders.add(inputView);
    PrintStream stdout = System.out;
    if (opts.has("l")) {
I guess I don't have a strong opinion on this but should we move this up? Otherwise messages prior would still show up on the console instead of log. As far as I can tell this would just be the "Running headless?" message but if there were more diagnostics in say the constructor for DbViewer then those would get misplaced.
Sure, that makes sense. I was trying to affect just batch operation, but if you want to adopt it generally, that makes perfect sense.

In cvs/objects/java/source/RunQueries.java #1 (review):
     mBatchFile = aBatchFile;
mLogFile = aLogFile;
Check tabs vs spacing
Will do.

—

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #1 (review), or mute the thread https://github.com/notifications/unsubscribe-auth/ADXRHpmilmasulJ5AlMUFsq7R2SdPR3Eks5qtCzOgaJpZM4JXQ0f.

…parsing of the options. In addtion add the option to print the JAVA_HOME for the JVM that is currently running. This helps us create a run GCAM wrapper script to set up paths to find the JVM dynamic library to run GCAM.

pralitp · 2016-09-23T20:27:51Z

I used 1.8 because that's what I'm running on my Mac. I would be surprised if the code is sensitive to that, but who knows.

Yea, I wouldn't expect that to change compilation (although I have run into that in the past). This was from the perspective of the .jar files that get committed on the idea of we can just commit them using an older version of Java and not have to worry about users having to recompile them again for themselves.

With both of these, I don't see changes that I would have made. Could this be due to some difference between the public github repo and your internal one?

I think this was just because I shared these changes with you via patch. Since I have them on anther branch let's just revert them here.

* Some code clean up. * Revert some changes that will be made elsewhere. * Build jars for java 1.6 (Note this means we need to use joptsimple v4.9)

… on what it is and how to use it. In addtion we will default it to save the log in the GCAM "logs" directory.

pralitp · 2016-10-03T20:07:08Z

Rich, we've decided to keep the logistical hurdles as low as we can we will take Github pull requests to their own feature branch and merge it into the master on our Bitbucket server. This way we can automatically (eventually automated at least) sync changes from Bitbucket to Github in just one direction.

Modifications to support logfile for MI output and other minor cleanup

73cadd2

pralitp reviewed Sep 23, 2016

View reviewed changes

pralitp added 2 commits September 23, 2016 15:05

Address comments from pull request:

d0e15ad

* Some code clean up. * Revert some changes that will be made elsewhere. * Build jars for java 1.6 (Note this means we need to use joptsimple v4.9)

Add the batch-logfile to the XMLDB.properties file including comments…

6658d7f

… on what it is and how to use it. In addtion we will default it to save the log in the GCAM "logs" directory.

rplzzz changed the base branch from master to feature/MI-logging October 3, 2016 19:44

pralitp merged commit 40aa862 into JGCRI:feature/MI-logging Oct 3, 2016

robbieorvis mentioned this pull request Oct 18, 2019

GCAM-USA target finder failing to calibrate in 2010 #76

Closed

ouyang363 mentioned this pull request Aug 30, 2022

Errors in running GCAM v6.0 for GHG constraints #232

Open

bfleming mentioned this pull request Mar 16, 2023

GCAM 6.0 build errors - boost fusion #274

Open

dobrien13 mentioned this pull request Mar 8, 2024

Running GCAM core with broken out region #400

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modifications to support logfile for MI output and other minor cleanup #1

Modifications to support logfile for MI output and other minor cleanup #1

rjplevin commented Jul 28, 2016

pralitp left a comment

pralitp Sep 23, 2016 •

edited

Loading

pralitp Sep 23, 2016

pralitp Sep 23, 2016

pralitp Sep 23, 2016

pralitp Sep 23, 2016

pralitp Sep 23, 2016

rjplevin commented Sep 23, 2016

pralitp commented Sep 23, 2016 •

edited

Loading

pralitp commented Oct 3, 2016

Modifications to support logfile for MI output and other minor cleanup #1

Modifications to support logfile for MI output and other minor cleanup #1

Conversation

rjplevin commented Jul 28, 2016

pralitp left a comment

Choose a reason for hiding this comment

pralitp Sep 23, 2016 • edited Loading

Choose a reason for hiding this comment

pralitp Sep 23, 2016

Choose a reason for hiding this comment

pralitp Sep 23, 2016

Choose a reason for hiding this comment

pralitp Sep 23, 2016

Choose a reason for hiding this comment

pralitp Sep 23, 2016

Choose a reason for hiding this comment

pralitp Sep 23, 2016

Choose a reason for hiding this comment

rjplevin commented Sep 23, 2016

pralitp commented Sep 23, 2016 • edited Loading

pralitp commented Oct 3, 2016

pralitp Sep 23, 2016 •

edited

Loading

pralitp commented Sep 23, 2016 •

edited

Loading