Skip to main content

Bug reports

90 posts / 0 new
Last post
mpyat2
mpyat2's picture
VeLaBaseVisitor.java missing in the repo?

Hi David,

I've tried to build the last revision (using ant) and have got errors connected with ExpressionVisitor.java (see attachment). It seems that VeLaBaseVisitor.java is missing. Could you please help to solve the problem?

Best regards,

Max

File upload: 
David Benn
David Benn's picture
VeLaBaseVisitor.java missing in the repo?

Hi Max

This source file is generated from VeLa.g4, a grammar for the VeLa language I've created for VStar.

build.xml is fine to build vstar.jar for plugin development, but I should update build-dev.xml to generate ANTLR artefacts too.

I've created a ticket: https://sourceforge.net/p/vstar/bugs-and-features/655/

David

mpyat2
mpyat2's picture
VStar build

Hi David,

Thank you for clarification. I've noted several .java files created while building, so now I understand from what they appeared.

What do you recommend to enable building VStar from the sources? (some additional manual tasks before running 'ant'?)

Best regards,

Max

David Benn
David Benn's picture
VStar build

Hi Max

No worries.

If your current directory is the location of build.xml, you can type this to see all Ant targets:

ant -p

Buildfile: <your-dir>/build.xml

Ant build file for VStar.

Main targets:
antlr4 Generate Java source code from ANTLR VeLa grammar.
bash Create a standalone distribution archive for bash
clean Clean up
compile_src Compile the source
compile_ut Compile the test code
debug Compile the source for debug
dist Generate the distribution
javadoc Javadoc generation
mac Create a standalone distribution archive for Mac
manifest Creates a manifest file for VStar
run Run VStar GUI
run_debug Run debug VStar GUI
test Run unit tests
win32 Create a standalone distribution archive for Win32
Default target: dist

Notice that the default target is dist, so these two commands:

ant dist
ant

are equivalent and will yield vstar.jar

Nothing further should be required to build VStar from sources. Please let me know if this is not the case.

win32, bash, and mac targets yield Windows, Linux (or anything with bash) and Mac archives (zip in first two cases, DMG in latter but an Apple Developer ID is required for Mac).

Does that help?

David

mpyat2
mpyat2's picture
Building VStar from sourses

Hi David,

there is build-dev.xml file in the trunk of the repository.

I tried to build VStar with the following command:

C:\personal\Projects\VStar>ant -f build-dev.xml

under Windows 10.

The build was unsuccessful, javac complains about missing "VeLaBaseVisitor".

I also tried running ant with antlr4 target, build was SUCCESSFULL. However, there was no VeLaBaseVisitor.java generated (along with VeLaVisitor.java). See attachment for the list of java files generated as a result of "ant -f build-dev.xml antlr4".

Best regards,

Max.

 

File upload: 
mpyat2
mpyat2's picture
Building VStar

Hi David,

I've eventually managed to build the program. I've changed line 81 in buids-dev.xml from

"<arg value="-no-visitor" />"

to 

<arg value="-visitor" />. 

However, I did not understand what I've done :) Could you please explain what does in mean (why the generation of "visitor" files was switched off)?

Best regards,

Max

David Benn
David Benn's picture
Building VStar

Hi Max

Okay, thanks.

<arg value="-visitor" /> is what build.xml uses too.

I don't recall right now why build-dev.xml was using -no-visitor. That Ant build file has not been updated for quite awhile. Perhaps I should add the targets therein into build.xml so it's all in sync. The main benefits of build-dev.xml are for code coverage and static analysis.

The -visitor switch tells ANTLR to generate a base class for grammar visitor code. VStar subclasses this to create an abstract syntax tree from each VeLa parse. Such ASTs are then used by the VeLa interpreter.

David

mpyat2
mpyat2's picture
Build.xml

Hi David,

Strange, when I downloaded SVN (using TortoiseSVN under Windows 10) there was no "build.xml" downloaded, "build-dev.xml" only (that's why I tried this build file). However, I can see "build.xml" in the trunk while browsing the code on sourceforge.net site. So I've just downloaded build.xml manually. Build using "build.xml" failed:

BUILD FAILED
C:\Personal\SKY2\VStar\build.xml:32: taskdef class com.oracle.appbundler.AppBundlerTask cannot be found using the classloader AntClassLoader[]

Regards,

Max

P.S. It seems it is Mac-specific task.

mpyat2
mpyat2's picture
Building using build.xml: done!

Hooray! After removing (commenting out) Mac-specific <taskdef name="bundleapp" classname="com.oracle.appbundler.AppBundlerTask" classpath="${extlib}/appbundler-1.0.jar" /> I can build VStar under Windows 10 using build.xml!

 

David Benn
David Benn's picture
Building using build.xml: done!

Hi Max

Cool.

I've opened a ticket to add the appbundler task jar (which is purely for Mac .dmg building) to the VStar repo since it's just in my working copy right now. surprise

This was being maintained by Oracle but not now. See the ticket:

  https://sourceforge.net/p/vstar/bugs-and-features/656/

Please let me know about any other non-reproducible issues you find. This is good. We want to guard against me getting hit by a bus. I get a lot of buses. laugh

David

mpyat2
mpyat2's picture
Bug with AoV/DFT

Hi David,

It's about a bug I've mentioned in #46  ("java.lang.IllegalArgumentException: null" when one uses AoV after DFT and vice versa)

Here is a scenario:

1) Start VStar
2) Open TEST_ASAS-SN_V.txt (see attachment).
3) Analysis -> DC DFT Standard scan..., do the scan. (*)
4) Close plot pane.
4) Analysis -> AoV with period range..., Min=1, Max=1000, Res=0.1, Bins=10
5) Click any of Top Hits (in the plot or in the table).
6) You will get an exception.

and vice versa:

1) Start VStar
2) Open TEST_ASAS-SN_V.txt (see attachment).
3) Analysis -> AoV with period range..., Min=1, Max=1000, Res=0.1, Bins=10. Do the scan. (*)
4) Close plot pane.
5) Analysis -> DC DFT Standard scan..., do the scan.
6) Click a Top Hit (in the plot or in the table).
7) You will get an exception.

See also a picture attached.

Quite annoying error. It requires the program to restart. I have not investigated it yet. Probably it is worth to create a ticket?

Regards,

Max

David Benn
David Benn's picture
Bug with AoV/DFT

Hi Max

Thanks for this excellent detailed how-to-reproduce bug description.

Yes, definitely worth a ticket. Do you have a SourceForge account? If so, I can add you to the SF VStar project so you can add tickets, if you would like to.

David 

mpyat2
mpyat2's picture
SourceForge account

Hi David,

Yes, I do have a Source Forge account: https://sourceforge.net/u/mpyat2sf/profile/

Regards, Max

David Benn
David Benn's picture
SourceForge account

Hi Max

I've added you as a developer. That may only give you ticket update permissions vs create. Let me know and I'll fix if so.

David

mpyat2
mpyat2's picture
Creating a ticket

Hi David,

Sorry for the delay.

I've just successfully created a ticket (#657 java.lang.IllegalArgumentException when AoV is used after DFT and vice versa).

All works.

Regards,

Max

David Benn
David Benn's picture
Creating a ticket

Thanks Max!

David

mpyat2
mpyat2's picture
SVN Checkout problem under Windows and some other questions

Hi David,

It seems I figured out why my SVN checkout did not get all files from VStar SVN repository. There is a "VStar.app" folder with "Icon " file. It seems that "Icon " filename has a trailing space (or spaces). Probably it has a special meaning under Mac OS, unfortunately, in Windows filesystem (NTFS) such a name is causing problems (formally, it is possible to create a file having trailing spaces in its name however most programs get confused with it). This is why (probably) I did not see "build.xml" file along with some other documents (however, despite the error, all *.java code were imported in my local folder so I could build the program!). Previously I ignored this error however now I suspect that it is probably a cause of incomplete repository importing.

I also noted that you have commented out line 1164 in Mediator.java: 

// convertObsToHJD(starInfo);

so now VStar does not convert data to HJD automatically in case of mixed time scales. SVN message states that it is connected with "#595 Standalone HJD conversion plugin". Does it mean that in the future versions, a rule of conversion will be changed? 

Regarding DoubleField.getValue(): returning null in case of any error requires a caller to check for null values, there are places in the code where a caller does not check. It is not a very serious bug, anyway, a user will get a null exception. Yet I'll try to find those places (I hope...). IMHO, if DoubleField.getValue() does not catch VeLa exceptions, it looks somewhat inconsistent (IntegerField, for example, does not use VeLa interpreter and never throws VeLa exceptions, it always returns null on error)

Regards,

Max

P.S. Another option would be using VeLa in both cases and always throwing an exception if a value is invalid or is out of bounds. It could be even safer.

P.P.S. Sorry for tediousness. DoubleField.getValue() uses VeLa interpreter, IntegerField.detValue() does not. So you can do some tricks while input in DoubleFiled (calculations on-the-fly)  yet can't with IntegerField (see the second picture: value for "Bins" will not be evaluated while values for period and resolution will be. Well, it is a feature, not a bug :) Another feature is that VeLa will evaluate 1/3 as 0 (integer division) and will not accept "1/3." only "1/3.0"... Well, I went too far, never mind :)

David Benn
David Benn's picture
SVN Checkout problem under Windows and some other questions

Hi Max

Apologies again for the delay. It's been a tricky last few days.

Re: VStar.app, this is generated by ant mac. Some previous version must have been committed inadvertently. I have just deleted it it from svn. 

Re: HJD, yes, when the next version is released, HJD conversion will no longer be implicit to allow greater flexibility and transparency. There are a couple of relevant tickets for this, #595 as you say and #456, at least.

Good point re: IntegerField. Thanks. Can you create a ticket for this please? I'll address that. The lack of use of VeLa there is probably because at the time, only real numbers were supported, with integer, string, boolean types added later to the language. So, as a lower priority, TextField and TextArea could also make use of VeLa since it includes a string type (e.g. think of VeLa filters). So that would seem to be another ticket for both.

1/3 in VeLa does indeed perform integer division while 1/3.0 is a floating point operation. While there is a formal syntax onlin for VeLa I have not yet described formally or informally too much semantics.

Never too far Max. I welcome your questions, feedback, suggestions and coding suggestions.

David

mpyat2
mpyat2's picture
Stange behaviour of AAVSO Upload Format File Plugin

Hi David,

Could you check if the following bug is reproducible:

If uncertainty (mag err) in AAVSO Upload Format File = 0.0 and there is a record with the same magnitude in the input file, after loading the uncertainty of the record with zero error gets a value of the uncertainty of another record with the same magnitude.

An example data file is in the attachment plus screenshot.

Best regards,

Max

P.S. It seems I found a source of the error and a workaround.

I will describe it later, now I’m in a place with almost no Internet.

 

 

 

David Benn
David Benn's picture
Stange behaviour of AAVSO Upload Format File Plugin

Hi Max

Thanks. Nicely spotted! And a very important bug to find and fix!

The problem code is here I think, as you may have found:

String uncertaintyStr = fields[3].trim();
if (!isNA(uncertaintyStr)) {
double uncertainty = uncertaintyValueValidator
.validate(uncertaintyStr);
observation.getMagnitude().setUncertainty(uncertainty);
}

This would in fact not be a problem if it weren't for the fact that VStar uses the FlyWeight pattern, caching and sharing repeated ValidObservation member data to reduce memory footprint. Code like the above predates this.

So long as there is no sharing, this is fine, but not otherwise.

As it turns out, I have just found that there is other observation source code where this problem also exists.

Other than the Magnitude class, there are one or two other classes used by ValidObservation for which this may also be an issue.

I'm inclined to start by removing mutating methods like setUncertainty() to see what breaks (statically) and to move Magnitude and any other offending class (like DateInfo) to always force construction of new objects.

The joys of mutable objects and the benefits of purely functional languages (which Java is of course not)! I'm increasingly fond of saying that I am in favour of very strongly typed languages with compilers that save me from my own human stupidity.

David

mpyat2
mpyat2's picture
Hi David,

Hi David,

You are right, the problem is in a line where we update an uncertainty of a magnitude object associated with the observation object. We should set uncertainty to a newly created magnitude object and then assign this object to the observation. After this we should treat magnitude of the observation as read-only (because all magnitudes are in global cache, several observations may share the same magnitude object).

The same is true for other cached objects, like date info. It is unsafe to modify dates inplace, like JD to HJD converter does (if I recall correctly, I cannot check it in the code now).

IMHO, we can get rid of global magnitude and date caches, they allow us to save a memory somehow yet I’m not sure we really need them because they make us to treat cached objects read-only for safety.

Best regards, Max

David Benn
David Benn's picture
Options

Hi Max

Yes, disabling the cache is definitely another option.

In fact, I agree with what you are saying: it's a good option is to disable caching for mutable objects (like Magnitude, DateInfo) and leave it in place for immutable objects (numeric, enumerated values) since there are many of the latter and fewer of the former within ValidObservation objects.

I'd suggest that the best thing to do at the moment is to create a ticket and reflect for a bit though, even though I think we're converging on a good approach.

This should certainly be considered a high priority ticket.

Thanks.

David

David Benn
David Benn's picture
Strange behaviour of AAVSO Upload Format File Plugin

A fix for this has been committed (see ValidObservation) and minimally tested so far. For example, the original bug you reported with supplied test data file now behaves as expected.

David

mpyat2
mpyat2's picture
Ticket

I would create the ticket but on Tuesday. Now I’m in a rural area with very slow Internet and a smartphone as the only option to connect.

David Benn
David Benn's picture
Ticket

No worries Max.

I've just created https://sourceforge.net/p/vstar/bugs-and-features/660/ and I'm working on the fix.There is measured benefit (from performance analysis in the past) in caching so I'd prefer not to throw the baby out with the bathwater so to speak, but if caching remains, the possibility of mutatability must be eradicated.

I'm a little embarassed by this bug.

It's wonderful to have more people with eyes on the code now (e.g. you, Cliff, Dave). I welcome the scrutiny and forum interactions.

Slightly off topic, but more broadly, I find that there is not enough open honest conversation in many areas of life these days.

Thanks again.

David

mpyat2
mpyat2's picture
creating new mag. object

Hi David,

You mentioned a possibility of creation a new object when magnitude object is changed: it will solve the problem however I suspect it could lead to orphan objects in the cache and memory loss. This reduces a usefulness of the cache...

Regards, Max

David Benn
David Benn's picture
orphan objects

Hi Max

That's right, there will be orphan objects, and even now that's the case but orphaned (unreferenced) objects will be garbage collected over time.

David

mpyat2
mpyat2's picture
orphan objects

Hi David,

I meant that if we create a new Magnitude object instead of an old one to be replaced and set it via setMagnitude(), the old object will not be connected to the observation however remains in the magnitudeCache. There are big chances that it will not be referenced by any other observations and there is a little chance that it will be reused again. It will be orphan in this sense, yet for the garbage collector, it is still referenced by the program (magnitudeCache) so it will not be removed from memory. We should remove it manually (check if it referenced by other observations and then delete if not).

I could be wrong, of course.

Regards,

Max

 

David Benn
David Benn's picture
orphan objects

Hi Max

I see what you mean. 

The cache is a "weak reference" cache which means that if no references to it exist outside of the cache, the GC is free to collect it.

Still, irrespective of this, I'm still trying to figure out the best course of action, i.e. whether to cache Magnitude objects or not. DateInfo usage was simple to sort out and the other objects are immutable.

We could just exclude Magnitude objects for now and revisit later.

David

mpyat2
mpyat2's picture
Weak reference

Hi David,

Thank you, now I understand the mechanics of this (I hope :) )

I have not been working with Java for many years so many things are new to me :)

Regards,

Max.

David Benn
David Benn's picture
Weak reference

No worries Max. There are many incidental intricacies but your knowledge of Java seems very sound to me. Thanks.

David

clkotnik
clkotnik's picture
Hi folks,

Hi folks,

I can offer my take on this problem.  In the limited maintenance work I have done with VStar, I found the biggest barrier for my work is the sophisticated software architecture embodied in the application.  While the architecture has many benefits, I find it a challenge to figure out what is going in the internals. 

In this particular case, while I have implemented code using magnitude objects I had no idea what was going on with the cache.  It seems to me that with the current generation of hardware, caching magnitudes would be better if abandoned in favor of correctness and simplicity.

Just my thoughts,

Cliff

David Benn
David Benn's picture
On Complexity

Hi Cliff

Thanks for weighing in.

Yes, I think your comment about complexity is fair. A decade of development has led to a lot of code and gobs of design pattern application. surprise I have a long-standing TODO list item (see ticket 99) to document the design at least at a high level, e.g. via UML. I may start a topic to capture some of this (and feedback) as well. The plugin architecture is intended to expose just enough to allow VStar to be extended, but I accept that even that is not transparent or simple enough.

Regarding caching, it's not so much about cache lines as memory footprint. However, I agree that correctness and simplicity rule here. Before this, I had fixed all but the Magnitude class by making any other cached object immutable. The various ways in which Magnitude is used internally and by plugin code is complex enough that for the next release at least, Magnitude objects will no longer be cached.

David

mpyat2
mpyat2's picture
Vstar does not close files after reading

Hi David,

I noticed that Vstar does not close files after reading (it prevents renaming/deleting them).
If another file is opened by the same plugin (or internal file reader) the first one is released however if multiple plugins are used, several files could be locked.
Many plugins use BufferedReader (inside retrieveObservations()) which never gets closed after reading. It could be enclosed in try-with-resource block (as recommended by JDK documentation):

try (BufferedReader reader = new BufferedReader(
  new InputStreamReader(getInputStreams().get(0)))) {
//... reading the input ...
} catch (IOException e) {
throw new ObservationReadError("IO error while opening input stream: " +
  e.getLocalizedMessage());
}

This closes the input stream immediately after reading.

A solution is slightly more tricky if a plugin creates BufferedReader in a separate method yet it could be solved too.

Internal file reader requires a separate approach.

I've made some tests with a plugin I’m developing now, it seems this approach works.

Don’t you mind if I create a ticket?

Regards,
Max
 

David Benn
David Benn's picture
Vstar does not close files after reading

Hi Max

Please create a ticket.

I'm surprised I didn't do this in a try...finally block.

The opening and closing of files (and http streams) needs to be handled centrally within core VStar code rather than in each plugin, so will have to look at this closely.

David

mpyat2
mpyat2's picture
NewStarFromObSourcePluginTask.java

Hi David,

In NewStarFromObSourcePluginTask.java (Rev. 1642):
  in createObservationArtefacts() method (where retrieveObservations() is called):
why MessageBox.showErrorDialog() [line 297] is commented out? 
VStars swallows exceptions thrown inside plugin's retrieveObservations() (IO errors, for example) without any notification to a user which could be confusing.

Regards,
Max

David Benn
David Benn's picture
NewStarFromObSourcePluginTask and other nasties

Hi Max

That was not intended. There's a couple of other NewStar*Task classes in that package (used by the scripting API) that actually do report the exceptions.

Fixed it. Also added an else case that should have been there that prevents further action if no obs were loaded that would lead to further exceptions.

David 

carlosmtron
carlosmtron's picture
Vstar working from SourceForge but not from AAVSO webpage

Hi,

I'm Carlos from Argentina and I'm just learning about light curves analysis.

My operative system is Debian GNU/Linux with icedtea 1.8.3 and openjdk 11.0.4.

I have downloaded Vstar from https://www.aavso.org/vstar. When I initializes the program with 

javaws vstar.jnlp

 from terminal it works fina, but when I try to load a Star Data from AAVSO database I obtain an error:

javax.xml.parsers.FactoryConfigurationError: Provider for class javax.xml.parsers.DocumentBuilderFactory cannot be created

The same problem is running from Firefox.

However, with the same OS and java version, I downloaded VStar from SourceForge, run with

./VStar.sh

and now is runnig fine.

Do you have idea about why I can't run it from the web?

Best regards,

Carlos

David Benn
David Benn's picture
VStar working from SourceForge but not from AAVSO webpage

Hi Carlos

Sorry for the delay in response.

I'm not sure exactly what the problem is here but I would suggest instead using Oracle's Java runtime or Amazon's Corretto distribution: https://aws.amazon.com/corretto/

Can you tell us if that helps?

David

Pages

Log in to post comments
AAVSO 49 Bay State Rd. Cambridge, MA 02138 aavso@aavso.org 617-354-0484